feat: added focus styles for interactive elements #576
Conversation
Kyzyl-ool
requested review from
gorgeousvlad and
yuberdysheva
as code owners
|
Preview is ready. |
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
from
e353eeb to
56d663b
Compare
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
from
60b40aa to
28ebc66
Compare
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
6 times, most recently
from
664b9b9 to
d553cae
Compare
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
from
71ce27c to
53b970b
Compare
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
from
2fe79b8 to
228905d
Compare
| @@ -60,6 +60,12 @@ $block: '.#{$ns}content-layout-block'; | |||
| padding: $indentXL; | |||
| } | |||
|
|
|||
| &_theme { | |||
| &_dark { | |||
| --g-color-line-focus: var(--g-color-text-light-primary); | |||
There was a problem hiding this comment.
What color is default for --g-color-line-focus in uikit and why can't we use it?
There was a problem hiding this comment.
Ok, let's use default focus color in page-constructor
| @@ -251,7 +251,7 @@ export const SliderBlock = (props: WithChildren<SliderProps>) => { | |||
| return ( | |||
| // To have this key differ from keys used in renderDot function, added `-accessible-bar` part | |||
| <Fragment key={`${index}-accessible-bar`}> | |||
| {slidesCountByBreakpoint > 1 && ( | |||
| {slidesCountByBreakpoint > 0 && ( | |||
There was a problem hiding this comment.
why did you change the value? Could it break something?
There was a problem hiding this comment.
I change it because when we have slidesCountByBreakpoint === 1 the accessible bar didn't render and one was unable to find it vis screenreader
|
|
||
| return ( | ||
| slidesCountByBreakpoint > 0 && |
There was a problem hiding this comment.
Something wrong with a slider. I can tab trough cards and then if I click to the arrow it doesn't work right
https://disk.yandex.ru/i/7dRgmxUNMis7iA
There was a problem hiding this comment.
Yep, this is an issue I think came from react-slick, I didn't manage to find a solution better than wrap each slide with inert attribute. But it may be risky, as slides not visible for the user, will be unavailable for focus
| @@ -391,6 +391,7 @@ export const ReactPlayerBlock = React.forwardRef<ReactPlayerBlockHandler, ReactP | |||
| onProgress={onProgress} | |||
| onEnded={onEnded} | |||
| aria-label={ariaLabel} | |||
| previewTabIndex={-1} | |||
There was a problem hiding this comment.
It means that preview image will not be focusable with Tab. At the same time we have on preview image play button, which is also accessible via Tab. I thought it would be sufficient to have it focusable
| @@ -259,7 +264,15 @@ $block: '.#{$ns}hubspot-form'; | |||
| p { | |||
| color: var(--g-color-text-primary); | |||
| @include text-body-2(); | |||
| margin: 0 0 20px; | |||
| margin: 20px 0; | |||
There was a problem hiding this comment.
Can you use variable instead of 20px?
|
Introduced focus variables: |
| @@ -44,19 +44,24 @@ const FullscreenImage = (props: FullscreenImageProps) => { | |||
| onClick={openModal} | |||
| style={imageStyle} | |||
| /> | |||
| <div className={b('icon-wrapper', {visible: isMouseEnter})} onClick={openModal}> | |||
| <button | |||
There was a problem hiding this comment.
did we reset button styles?
| <div | ||
| className={b('media-wrapper')} | ||
| onClickCapture={openModal} | ||
| onMouseOver={onShowFullScreenButton} |
There was a problem hiding this comment.
:hover in css doesn't suite this case?
There was a problem hiding this comment.
suits, rewrote with pseudoclasses
| @@ -71,14 +71,14 @@ const PriceDetails = (props: PriceDetailsExtendProps) => { | |||
|
|
|||
| const getFoldableTitle = () => { | |||
| return ( | |||
| <div | |||
| <button | |||
There was a problem hiding this comment.
did we reset button styles?
Release-As: 4.24.0
Kyzyl-ool
force-pushed
the
feat/focus-styles
branch
from
262fc72 to
0c6794e
Compare
No description provided.