-
Notifications
You must be signed in to change notification settings - Fork 99
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
fix: improve a11y on some components #1045
Changes from 2 commits
161c7e2
d4de1b5
501d905
0fb21e6
29f9909
5b4bd69
024f502
95f0a6d
802f579
1120a04
ce4885c
f22f5f7
ef3dc2d
a5e5490
304feea
6a16d65
2e17d94
bc2144e
d2d22fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,6 +201,7 @@ export const TableColumnSetup = (props: TableColumnSetupProps) => { | |
|
||
return ( | ||
<div className={b(null, className)}> | ||
{/* It is used to handle clicks on the switcher control */} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that should not work this way we should pass handler via some context i guess to easily pass it to rendered component |
||
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} | ||
<div className={b('control')} ref={refControl} onClick={handleControlClick}> | ||
{switcher || ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,16 @@ $block: '.#{variables.$ns}user-avatar'; | |
background-position: center; | ||
background-size: cover; | ||
|
||
outline: none; | ||
|
||
&:focus { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably have SCSS mixin for it |
||
box-shadow: 0 0 0 2px var(--g-color-line-focus); | ||
} | ||
|
||
&:focus:not(:focus-visible) { | ||
box-shadow: none; | ||
} | ||
|
||
&_size { | ||
&_xs { | ||
width: 24px; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import React from 'react'; | ||
|
||
import {block} from '../utils/cn'; | ||
import {useActionHandlers} from '../utils/useActionHandlers'; | ||
|
||
import {SIZES} from './constants'; | ||
import type {UserAvatarSize} from './types'; | ||
|
@@ -34,9 +35,18 @@ export const UserAvatar = React.forwardRef<HTMLDivElement, UserAvatarProps>( | |
setIsErrored(false); | ||
}, [imgUrl]); | ||
|
||
const {onKeyDown} = useActionHandlers(onClick); | ||
|
||
return ( | ||
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions | ||
<div className={b({size}, className)} title={title} onClick={onClick} ref={ref}> | ||
<div | ||
role="button" | ||
tabIndex={0} | ||
onKeyDown={onKeyDown} | ||
className={b({size}, className)} | ||
title={title} | ||
onClick={onClick} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. onClick supposed to be removed from this component |
||
ref={ref} | ||
> | ||
<img | ||
className={b('figure')} | ||
width={SIZES[size]} | ||
|
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.
I'm not sure that it is true
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.
argeed
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.
And I think we should remove this behaviour in the next major (#600)