Skip to content
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

Handle focus on focusable formatters #2104

Merged
merged 6 commits into from
Jul 29, 2020
Merged

Conversation

amanmahajan7
Copy link
Contributor

https://www.w3.org/TR/wai-aria-practices/#gridNav_focus

For assistive technology users, the quality of experience when navigating a grid heavily depends on both what a cell contains and on where keyboard focus is set. For example, if a cell contains a button and a grid navigation key places focus on the cell instead of the button, screen readers announce the button label but do not tell users a button is present.

There are two optimal cell design and focus behavior combinations:

  • A cell contains one widget whose operation does not require arrow keys and grid navigation keys set focus on that widget. Examples of such widgets include link, button, menubutton, toggle button, radio button (not radio group), switch, and checkbox.
  • A cell contains text or a single graphic and grid navigation keys set focus on the cell.

https://www.loom.com/share/3a0e559f0ea04db98226c3169dffad04

@amanmahajan7 amanmahajan7 self-assigned this Jul 28, 2020
export function SelectCellFormatter({ value, tabIndex, isCellSelected, disabled, onChange }: SelectCellFormatterProps) {
const inputRef = useRef<HTMLInputElement>(null);
useEffect(() => {
if (isCellSelected) {
Copy link
Contributor Author

@amanmahajan7 amanmahajan7 Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User can handle focus however they like and event can be stopped from propagating for complex components.

Copy link
Contributor

@nstepien nstepien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just needs a changelog.

return (
<>
<div className="rdg-child-row-action-cross" />
{isDeleteSubRowEnabled && (
<div className="rdg-child-row-btn" onClick={onDeleteSubRow}>
<span
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add another element?
Also I think if you just set onClick={onDeleteSubRow} instead of handling onKeyDown manually it might work just as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise it looks like this

image

@amanmahajan7 amanmahajan7 merged commit 4f658c6 into canary Jul 29, 2020
@amanmahajan7 amanmahajan7 deleted the am-cell-content-focus branch July 29, 2020 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants