feat(PPDSC-2054): add feedback element and transition preset to slider, checkbox & radio button #160
Conversation
evgenitsn
added
the
ready for design review
evgenitsn
requested review from
jps,
Xin00163,
pdimova,
baburay23,
mutebg,
Vanals and
Srinivasan-Ramasamy
as code owners
…nsition-preset-to-slider-checkbox-radio
…nsition-preset-to-slider-checkbox-radio
src/slider/slider.tsx
Outdated
| @@ -117,6 +118,11 @@ const ThemelessSlider: React.FC<SliderProps> = ({ | |||
| data-testid={`${dataTestId}-thumb`} | |||
| overrides={overrides} | |||
| > | |||
| <StyledFeedback | |||
There was a problem hiding this comment.
Just to clarify that this feedback component unlike the one one the base sitch is layered above the thumb, right ?
There was a problem hiding this comment.
Actually I am not sure about that one. I haven't thought about it at all.
Now looking at the code I can see that we control with layering on the BaseSwitch using this right?
const STACKING_CONTEXT = {
feedback: '1',
input: '2',
};
I haven't seen any specific requirement for that, so maybe we want to align that layering for the slider to be the same as the BaseSwitch?
There was a problem hiding this comment.
My personal preference would be to get the Feedback component to work the same way across all components, including the layering as well.
But on the other hand, it looks like there is no need for such change now.
@mutebg We need to check how these z-indices will work inside a Layer for example
evgenitsn
added
ready for review
| Currently only Safari 15.4 supports :focus-visible. | ||
| For the previous versions of Safari, we will return true here which means that the focus ring on the elements which are using this function will be visible on mouse click. So this acts more like an incremental enhancement for Safari. For all other supported browsers this will work as expected because they have good support for :focus-visible. | ||
| */ | ||
| return true; |
There was a problem hiding this comment.
In which cases will it return false?
There was a problem hiding this comment.
False would be returned only in the try block if the element doesn't have this selector applied, i.e. if this is false e.target.matches(':focus-visible');
In the catch block the result will only be true. The catch block will only be triggered if the browser does not support :focus-visible which means that in this case it's better to just return true to toggle the focus ring for the targeted element every time.
…nsition-preset-to-slider-checkbox-radio
evgenitsn
deleted the
feat/PPDSC-2054-add-feedback-element-and-transition-preset-to-slider-checkbox-radio
branch
…r, checkbox & radio button (#160) * feat(PPDSC-2054): add feedback element and transition preset * feat(PPDSC-2054): added :focus-visible support * feat(PPDSC-2054): update feedback override for the audio player seek bar * feat(PPDSC-2054): update snapshots * feat(PPDSC-2054): polishing code * feat(PPDSC-2054): update components naming
PPDSC-2054
What
Added:
opacityChangepresetopacityChangeapplied on the feedback element on the slider thumbopacityChangeapplied on the checkbox and radio button (base-switch) feedback elementsFeedback preset is now extracted and reused in the components mentioned above.
This PR combines:
https://nidigitalsolutions.jira.com/browse/PPDSC-2054
and
https://nidigitalsolutions.jira.com/browse/PPDSC-1965
I have done:
I have tested manually:
Screen.Recording.2022-04-13.at.3.54.40.pm.mov
Screen.Recording.2022-04-13.at.3.55.22.pm.mov
Screen.Recording.2022-04-13.at.3.54.05.pm.mov