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

Accessibility: Improve keyboard accessibility in Collapse #59022

Merged
merged 1 commit into from Nov 22, 2022

Conversation

ashharrison90
Copy link
Contributor

What is this feature?

  • uses the correct semantic element (<button>) for the collapse header
  • applies some styling to make sure it still looks the same

Why do we need this feature?

  • better keyboard accessibility across grafana

Who is this feature for?

  • everyone! 🙌

Which issue(s) does this PR fix?:

Fixes #59021

Special notes for your reviewer:

@@ -145,10 +145,10 @@ export const Collapse = ({

return (
<div className={panelClass}>
<div className={headerClass} onClick={onClickToggle}>
<button type="button" className={cx(buttonStyles, headerClass)} onClick={onClickToggle}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like a similar button is used more than once, should we have a separate component for it? Maybe a modified version of IconButton?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Clarity-89 don't quite understand - do you mean creating a component for <button type="button ...>? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, but also with the clear button styles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah got it. yeah that could have value... 🤔 lemme have a think about implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gonna merge for now whilst we think about an implementation for exposing an unstyled button.

debating between just exposing a new Button variant (unstyled), or a whole new component UnstyledButton. neither of which feel super nice tbh. wondering whether we shouldn't expose this as although it's being used quite a bit internally, i don't think we'd want to expose it externally as part of @grafana/ui? idk, any thoughts? 🤔

@ashharrison90 ashharrison90 merged commit 19e97a1 into main Nov 22, 2022
@ashharrison90 ashharrison90 deleted the ash/59021 branch November 22, 2022 11:19
grafanabot pushed a commit that referenced this pull request Nov 22, 2022
fix keyboard accessibility in Collapse

(cherry picked from commit 19e97a1)
ashharrison90 added a commit that referenced this pull request Nov 22, 2022
…59097)

Accessibility: Improve keyboard accessibility in `Collapse` (#59022)

fix keyboard accessibility in Collapse

(cherry picked from commit 19e97a1)

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>
GuYounes pushed a commit to paul-wurth/BIXpert that referenced this pull request Feb 8, 2023
…rafana#59097)

Accessibility: Improve keyboard accessibility in `Collapse` (grafana#59022)

fix keyboard accessibility in Collapse

(cherry picked from commit 19e97a1)

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A11y: Fix keyboard accessibility in packages/grafana-ui/src/components/Collapse/Collapse.tsx
3 participants