feat: improving a11y #1070
feat: improving a11y #1070
Conversation
Kyzyl-ool
requested review from
LakeVostok,
axtk,
amje,
ValeraS and
korvin89
as code owners
|
Preview is ready. |
src/components/Dialog/i18n/ru.json
Outdated
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "close": "Закрыть" | |||
src/components/Link/Link.tsx
Outdated
| @@ -75,6 +75,7 @@ export const Link = React.forwardRef<HTMLElement, LinkProps>(function Link( | |||
| const relProp = target === '_blank' && !rel ? 'noopener noreferrer' : rel; | |||
|
|
|||
| return ( | |||
| // children is passed in commonProps | |||
There was a problem hiding this comment.
may be if we pass children explicitly this will pass this lint rule?
There was a problem hiding this comment.
yes, you were right
| @@ -56,11 +56,27 @@ $block: '.#{variables.$ns}persona'; | |||
| } | |||
| } | |||
|
|
|||
| &_clickable { | |||
| #{$block}__main { | |||
There was a problem hiding this comment.
Move it to element itself, to have all styles for element in single place
#{$block} {
// ...
&__main {
#{$block}_clickable & {
//
}
}
// ...
}There was a problem hiding this comment.
it was not obvious, but seems useful, thank you
| <div className={b('text')}>{children}</div> | ||
| </div> | ||
| {React.createElement( | ||
| clickable ? 'button' : 'div', |
There was a problem hiding this comment.
const MainComponent = clickable ? 'button' : 'div';
<MainComponent/>| @@ -136,5 +155,19 @@ $block: '.#{variables.$ns}persona'; | |||
| &:hover { | |||
| color: var(--g-color-text-primary); | |||
| } | |||
|
|
|||
| .yc-icon { | |||
There was a problem hiding this comment.
add className to icon component
| @@ -58,3 +58,4 @@ export {useOnFocusOutside} from './utils/useOnFocusOutside'; | |||
| export * from './utils/interactions'; | |||
| export * from './utils/xpath'; | |||
| export {getLayersCount} from './utils/LayerManager'; | |||
| export {getUniqId} from './utils/common'; | |||
There was a problem hiding this comment.
Exporting useUniqId hook is not enough?
There was a problem hiding this comment.
No, as one may use class components
| @@ -137,6 +137,7 @@ const DropdownMenu = <T,>({ | |||
|
|
|||
| return ( | |||
| <DropdownMenuContext.Provider value={contextValue}> | |||
| {/*as this div has Button component as child, clicking on it one fires onClick of this div on bubbling*/} | |||
There was a problem hiding this comment.
i guess we should make breaking change in future and always add onClick to toggle control
There was a problem hiding this comment.
if button has onClick handler also, it may lead to double-handling of click event. Should be accurate there
Kyzyl-ool
force-pushed
the
feat/a11y-improvements-2
branch
from
620280a to
b46148c
Compare
Kyzyl-ool
force-pushed
the
feat/a11y-improvements-2
branch
from
b46148c to
b0bd78a
Compare
Kyzyl-ool
force-pushed
the
feat/a11y-improvements-2
branch
from
b0bd78a to
f63ee78
Compare
closes #685
closes #966