Skip to content
This repository was archived by the owner on Nov 10, 2020. It is now read-only.

Several UI fixes #136

Merged
merged 25 commits into from
Aug 6, 2017
Merged

Several UI fixes #136

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
887ad14
Fixed overflow issues
DRSDavidSoft Aug 1, 2017
37b24b2
Added a fade effect instead of immediate cut
DRSDavidSoft Aug 1, 2017
d02f88a
Modified excessive use of box-shadow in item cards
DRSDavidSoft Aug 1, 2017
0f93b74
Modified user_compact
DRSDavidSoft Aug 1, 2017
8f51bb2
Tuned log out button's position
DRSDavidSoft Aug 1, 2017
ba6abc7
Removed unnecessary margin
DRSDavidSoft Aug 1, 2017
21f01fa
Tuned spacing to meet the description requirements
DRSDavidSoft Aug 1, 2017
2ced05c
Added box-shadow to user-card
DRSDavidSoft Aug 1, 2017
c7e72dd
Fixes the close icon `x`
DRSDavidSoft Aug 1, 2017
cd0f64d
Merge pull request #1 from DRSDavidSoft/patch-1
DRSDavidSoft Aug 1, 2017
f178ffc
Updated login.js
DRSDavidSoft Aug 1, 2017
491319a
Tuned user-card spacing
DRSDavidSoft Aug 1, 2017
cb678de
Added a relative position to item_container
DRSDavidSoft Aug 1, 2017
69e51c1
Added padding
DRSDavidSoft Aug 1, 2017
d6cda23
Fixed `react/jsx-first-prop-new-line` issue
DRSDavidSoft Aug 2, 2017
c4b06a8
Fixed `no-trailing-spaces`
DRSDavidSoft Aug 2, 2017
028e14b
Changed pixel unit to rem
DRSDavidSoft Aug 3, 2017
e1004f5
Changed `left` property according to review request
DRSDavidSoft Aug 3, 2017
5bac778
Added `user-select: none` to bottom container
DRSDavidSoft Aug 3, 2017
c23a557
Merge branch 'master' into master
tahnik Aug 6, 2017
02fe1e8
Restoring column styles. Moving from rem to px is not a good idea
tahnik Aug 6, 2017
91420c9
Made the items_container bigger as @DRSDavidSoft suggested
tahnik Aug 6, 2017
cf8a773
Not sure why you're doing this. This is causing the scroll bar to be …
tahnik Aug 6, 2017
08e477f
Box shadow has been verified by other tester. Let's not change it
tahnik Aug 6, 2017
b73ca8c
Updated package version
tahnik Aug 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/src/main/js/components/auth/login.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ class Login extends Component {
<div className="auth_image" >
<img alt="devrant" src="./res/images/devrant_sidebar.png" />
</div>
<div className="auth_form" >
<div className="auth_form"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need this, but this doesn't work for some reason. Can you check?

Edit: The reason is this is a div, not a form. So onSubmit won't be triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not change it to a form? It makes sense
Otherwise, we should write a handler on both fields to check e.which == 13 on keypress, which I don't recommend

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have recently fixed it using keyDownEvent. so don't worry about this one.

onSubmit={() => this.props.login(
this.state.username,
this.state.password,
)}
>
<input
value={this.state.username}
onChange={e => this.setState({ username: e.target.value })}
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/css/modules/bottom_bar.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
font-size: 1.5rem
display: flex
background-color: #40415A
padding: 0.5rem
padding: 0.8rem 1rem
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dacexi can you review this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RekkyRek I'd be grateful if you did!

.addMention
display: flex
align-items: center
Expand Down
12 changes: 7 additions & 5 deletions app/src/main/res/css/modules/column.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@
margin-top: 5rem
font-size: 1.3rem
.items_container
overflow-x: hidden
overflow-y: scroll
overflow-x: hidden
position: relative
overflow: auto
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes some rants cause overflow-x issues. It would be better to keep it the way it is.

Copy link
Contributor Author

@DRSDavidSoft DRSDavidSoft Aug 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it's not. I think I know what you're referring to. Instead of overflow-x, this should be done:
https://stackoverflow.com/q/3058866/1454514

or better - https://stackoverflow.com/a/18706058/1454514

TL;DR: word-wrap: break-word; & word-break: break-all;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add word-break: break-all; but it is still better to keep overflow-x hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tahnik I hear you, but it causes problems with box-shadow. If it's going to be hidden, there should be enough padding for the box-shadow IMHO.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DRSDavidSoft break-all causes some other problems. For example, it also breaks words which look horrible. box-shadow looks fine now so have a look.

overflow-x: visible
transform: translateZ(0)
-webkit-transform: translateZ(0)
padding: 0rem 0rem 1rem 0rem
height: calc(100vh - 2.8rem - 1.5rem)
padding: 0 10px
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing the column not to align with the topbar. Is this what you intended? I don't think that looks nice. Maybe @Dacexi can confirm it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tahnik Can you post a screenshot from before and after please?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it in another commit.

height: 100vh
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't make it 100vh please. Keep some padding in the bottom. This can cause side effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tahnik Tested with some other situations, I agree.

.item_column
position: relative
.column_topbar
height: 30px
overflow: hidden
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/css/modules/comments.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
button
flex: 2
.comments_container
padding: 0 0.5rem
h4
text-align: center
color: white
Expand Down
7 changes: 7 additions & 0 deletions app/src/main/res/css/modules/common.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ body
display: flex
justify-content: center
height: 100%
&:after
content: ''
position: absolute
left: 0
bottom: 0
width: 100%
background: linear-gradient(to top,#54556e 0%,rgba(0,0,0,0) 100%)

.fade-enter
opacity: 0.01
Expand Down
15 changes: 8 additions & 7 deletions app/src/main/res/css/modules/compact_user_card.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

.user_compact
//background: url(./../images/empty_avatar.png)
background-size: 100% 100% !important
background-size: cover !important
background-repeat: no-repeat !important
position: relative
display: flex
align-items: center
justify-content: center
flex-direction: column
padding: 1rem
padding-bottom: 2rem
.user_bg_tint
position: absolute
top: 0px
Expand Down Expand Up @@ -51,8 +52,8 @@
cursor: pointer
.logout
position: absolute
bottom: 5px
left: 10px
bottom: 10px
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing the shadow in the back to misalign

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to 0.25rem

Copy link
Contributor Author

@DRSDavidSoft DRSDavidSoft Aug 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Initially I used px instead of rem because the previous commit also used it.
I suppose most of the units should be using rem here, right?

left: 1.5rem
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to 0.5rem instead. Looks a bit odd when it's so high up.

Copy link
Contributor Author

@DRSDavidSoft DRSDavidSoft Aug 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to the left property? I set it to 0.5rem, but could you comment on the suitable values for both left and bottom properties?

font-size: 1.5rem
z-index: 300
display: flex
Expand All @@ -63,12 +64,12 @@
i
text-shadow: 0px 0px 4px rgba(0,0,0,0.4)
&:hover::after
margin-left: 0.2rem
margin-left: 0.4rem
opacity: 1
text-shadow: 0px 0px 4px rgba(0,0,0,0.4)
&::after
content: attr(data-text)
font-size: 1rem
margin-left: -1.4rem
font-size: 1.1rem
margin-left: -0.6rem
opacity: 0
transition: opacity 0.2s, margin-left 0.2s
transition: opacity 0.2s, margin-left 0.2s
3 changes: 1 addition & 2 deletions app/src/main/res/css/modules/item_card.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
margin-bottom: 1rem
border-radius: 3px
transition: all 0.2s ease
margin-right: 0.5rem
position: relative
.body_container
.top_container
Expand Down Expand Up @@ -41,7 +40,7 @@
width: 100%
.item_card.shadow
&:hover
box-shadow: 0 10px 20px rgba(0,0,0,0.19), 0 6px 6px rgba(0,0,0,0.23)
box-shadow: 0 2px 0px rgba(0, 0, 0, 0.19), 0 3px 4px rgba(0, 0, 0, 0.23)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it's looking nice either. Looks more like a black border than a shadow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tahnik, OK, let's keep the previous one, but please make sure it has enough padding.

.body_container
&:hover
cursor: pointer
Expand Down
13 changes: 8 additions & 5 deletions app/src/main/res/css/modules/user_card.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
.user_card
position: absolute
width: 25rem
width: calc(100% - 2rem)
background-size: cover !important
background-repeat: no-repeat !important
padding: 1rem 1rem 0rem 1rem
box-shadow: 0 2px 0px rgba(0, 0, 0, 0.19), 0 3px 4px rgba(0, 0, 0, 0.23)
padding: 1rem 1rem 2rem 1rem
transform: translate(-1rem, -1rem)
z-index: 99
button
box-shadow: none
&:hover
box-shadow: none
&::before
&:before
content: " "
background: rgba(64, 65, 90, 0.9)
position: absolute
Expand All @@ -21,7 +22,8 @@
.close
position: absolute
width: 1.2rem
height: 1.3rem
height: 1.2rem
line-height: 1.3rem
right: 5px
top: 5px
border-radius: 100%
Expand All @@ -39,7 +41,8 @@
i
color: black
.user_openprofile
width: 27rem
position: absolute;
width: calc(100%)
transform: translateX(-1rem)
margin: 0px
text-shadow: 0px 0px 2px rgba(0,0,0,0.3)
Expand Down