-
Notifications
You must be signed in to change notification settings - Fork 701
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
Remove javascript scrollbar library #429
Conversation
I think there were some issues discussed on the channel, but if we end up being able to get rid of that plugin, that's really cool! 🎉 Will review when I get some free time. |
I like that, works really well! Good job 👍 Styling could be a bit better (at least color match the old tse one and a transition), but that can go with another PR. |
overflow-y: scroll; | ||
.scrollable { | ||
overflow: auto; | ||
visibility: hidden; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has an unfortunate side effect, so I suggest removing fiddling with scrollbar visibility and letting the OS decide: scrollbars are set up to always show up on my OS, and this change effectively ignores this setup. Essentially, this is an accessibility issue (accessibility is not only about screen readers ;-) ).
One minor comment (disabling the display-on-hover) and then I'll be +1 on that PR :-) |
@astorije That's not exactly minor. Windows scrollbars are ugly in all browsers (except Chrome where we style them), and I'm sure users would want to get rid of them. |
Right, I understand. However, I believe this should be a recipe in the CSS Mods page because, as it is in this PR, it contradicts OS settings in OSes that offer system-wide control of the scrollbars. |
@astorije I've removed the hover stuff. |
Great @xPaw, 👍! |
Remove javascript scrollbar library, use hover appearing scrollbar instead
Removes the tse-scrollbar library. Makes the channel list and user list only have scrollbar visible when hovering over the container.
This library was used in the channel list only, it is sometimes quite laggy on mobile.