New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use default cursor on dummy scrollbars and make them 15px wide/tall #15275

Merged
merged 1 commit into from Aug 12, 2017

Conversation

Projects
None yet
4 participants
@as-cii
Member

as-cii commented Aug 12, 2017

Fixes #15166.

This restores scrollbars to adopt the styling they had prior to #13880, as you can see in the below CSS excerpt from Atom 1.18.0.

.horizontal-scrollbar {
position: absolute;
left: 0;
right: 0;
bottom: 0;
height: 15px;
overflow-x: auto;
overflow-y: hidden;
z-index: 3;
cursor: default;
.scrollbar-content {
height: 15px;
}
}
.vertical-scrollbar {
position: absolute;
top: 0;
right: 0;
bottom: 0;
width: 15px;
overflow-x: hidden;
overflow-y: auto;
z-index: 3;
cursor: default;
}

@nathansobo: do you recall the reason why changed the styling to 20px? I couldn't find an explanation on the commit that introduced them, so I went ahead and created this pull-request.

Also, /cc: @ungb and @Ben3eeE for help on testing this and making sure it restores the original behavior.

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Aug 12, 2017

Member

LGTM 🚢

Member

Ben3eeE commented Aug 12, 2017

LGTM 🚢

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Aug 12, 2017

Contributor

I honestly don't recall. Sorry. Let's just roll with this.

Contributor

nathansobo commented Aug 12, 2017

I honestly don't recall. Sorry. Let's just roll with this.

@nathansobo nathansobo merged commit d8a8b03 into master Aug 12, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nathansobo nathansobo deleted the as-fix-scrollbar-cursor branch Aug 12, 2017

nathansobo added a commit that referenced this pull request Aug 12, 2017

Merge pull request #15275 from atom/as-fix-scrollbar-cursor
Use default cursor on dummy scrollbars and make them 15px wide/tall

nathansobo added a commit that referenced this pull request Aug 12, 2017

Merge pull request #15275 from atom/as-fix-scrollbar-cursor
Use default cursor on dummy scrollbars and make them 15px wide/tall
@pfitzseb

This comment has been minimized.

Show comment
Hide comment
@pfitzseb

pfitzseb Aug 16, 2017

Would you consider making this dependant on the scrollbar's visibility?
For very small editors the (invisible) scrollbars take up most of the editor's area, hence the cursor will be incorrect for most of it:
image

pfitzseb commented Aug 16, 2017

Would you consider making this dependant on the scrollbar's visibility?
For very small editors the (invisible) scrollbars take up most of the editor's area, hence the cursor will be incorrect for most of it:
image

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Aug 16, 2017

Member

@pfitzseb We have an open issue for scrollbar divs taking up space even when not visible, #14661. Can you please comment there? Thank you 🙇

Member

Ben3eeE commented Aug 16, 2017

@pfitzseb We have an open issue for scrollbar divs taking up space even when not visible, #14661. Can you please comment there? Thank you 🙇

@pfitzseb

This comment has been minimized.

Show comment
Hide comment
@pfitzseb

pfitzseb Aug 16, 2017

Thanks for pointing me there :)

pfitzseb commented Aug 16, 2017

Thanks for pointing me there :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment