Fix: navigation buttons shouldn't cover scrollbar #838
Conversation
|
Verified that @ConradJChan has signed the CLA. Thanks for the pull request! |
|
@ConradJChan, can you attach before and after screenshots so we can see the change? |
src/lib/_navigation.scss
Outdated
| @@ -39,6 +39,11 @@ $navigationBtnWidth: 50px; | |||
| box-shadow: none; | |||
| outline: none; | |||
| } | |||
|
|
|||
| & > svg { | |||
| margin-left: -5px; | |||
There was a problem hiding this comment.
Is this due to spacing built into the SVG?
There was a problem hiding this comment.
This is to center the SVG within the now 40px button, stand by for screenshots
src/lib/_navigation.scss
Outdated
| @@ -52,9 +57,10 @@ $navigationBtnWidth: 50px; | |||
| .bp-navigate-left { | |||
| border-radius: 0 2px 2px 0; | |||
| left: 0; | |||
src/lib/_navigation.scss
Outdated
| @@ -1,4 +1,4 @@ | |||
| $navigationBtnWidth: 50px; | |||
| $navigationBtnWidth: 40px; | |||
There was a problem hiding this comment.
Why reduce the button width? What if you left everything the same, but changed the left and right of .bp-navigate-left and .bp-navigate-right, would that work?
There was a problem hiding this comment.
Yeah, you're right I could keep the button width the same, I guess I was just trying to keep the button about in the same place as it ever was (though what is 5px vs 10px really)
|
Before: After: |
|
In addressing @DanDeMicco's comment, I'm keeping the button the same size now. I've also increased the offset to 20px because across the different browsers the scroll bar width is 17px, so just rounding up to 20 to be safe Now looks like this: Thoughts? |
src/lib/_navigation.scss
Outdated
| @@ -39,6 +39,10 @@ $navigationBtnWidth: 50px; | |||
| box-shadow: none; | |||
| outline: none; | |||
| } | |||
|
|
|||
| & > svg { | |||
There was a problem hiding this comment.
Is this still necessary?
There was a problem hiding this comment.
nope it is not, will remove
Made the navigation buttons skinner, in particular the next file navigation button, in order to allow room for the preview content scrollbar to be clickable