feat(Provider): set scrollbar styles#1223
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
Codecov Report
Continue to review full report at Codecov.
|
| // `ref` is a mutable object | ||
| ref.current = node | ||
| // The `current` property is defined as readonly, however it's a valid way because `ref` is a mutable object | ||
| ;(ref as React.MutableRefObject<N>).current = node |
There was a problem hiding this comment.
Nope and this is sad: prettier/prettier#4630 😭
There was a problem hiding this comment.
https://stardust-ui.github.io/react/shorthand-props
The same case, actually 😨
There was a problem hiding this comment.
Ok, just to mention I hate this, especially that we are not using ; at all... Sad day for me :D
There was a problem hiding this comment.
Even worse with the provided example, it is used in our guides... :\
| scrollbarHeight: pxToRem(16), | ||
| scrollbarWidth: pxToRem(16), | ||
|
|
||
| scrollbarThumbBackgroundColor: siteVariables.gray06, |
There was a problem hiding this comment.
Oops, base theme doesn't have gray06 and gray04 :S
There was a problem hiding this comment.
Definitely, I initially overlooked the path of styles, now they are in correct place.
P.S. Motivation to add them only to Teams theme:
- one of the frequent questions in SUI: "How I can disable custom scrollbars?"
- this feature is not cross-browser
There was a problem hiding this comment.
I would really love to see this as part of base theme, but I see your points. Let's think about this in the future. The base theme's provider should have custom scrollbar by default, at least in my opinion. Agreed that we should have way of enabling or disabling this behavior as well. The main reason for this is that, I don't see much different ways of defining the scrollbar, more over we can customize it trough it's variables.
miroslavstastny
left a comment
miroslavstastny
left a comment
There was a problem hiding this comment.
Changes look good, but are not covered by any test. Would it make sense to add a screener test for that?
…com/stardust-ui/react into feat/style-scrollbar # Conflicts: # CHANGELOG.md
3 participants
This adds styles for scrollbar to Teams theme. I defined styles in
ProviderBoxinstead ofstaticStylesto avoid collisions.