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

[EuiConfirmModal] Title should be wrapped in an HTML heading if provided as a string #5276

Closed
Tracked by #5265
1Copenut opened this issue Oct 7, 2021 · 10 comments · Fixed by #5494
Closed
Tracked by #5265

Comments

@1Copenut
Copy link
Contributor

1Copenut commented Oct 7, 2021

Steps to reproduce

  1. Open Kibana instance
  2. Open a modal like the screenshots below
  3. Evaluate with Dev Tools and verify there is text that is styled like a heading, but is semantically a simple DIV

Discard changes to dashboard modal


Clone dashboard modal


Create new tag modal

Actual Result

  • The modals have heading text that has a visual hierarchy that does not match its semantic hierarchy. Text that looks like a heading must be marked up as a heading to convey that importance to all users.

Expected Result

  • Text keeps the current styling, but is marked up as an H2

Kibana Version:
v8.0.0

OS:
MacOS Big Sur

Browser:
Chrome

Relevant WCAG Criteria: Understanding Success Criterion 1.3.1: Info and Relationships - Level A

@elasticmachine
Copy link
Collaborator

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@1Copenut 1Copenut changed the title [COGNITION]: [COGNITION]: Text that appears like a heading must be marked up as a heading Oct 7, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/kibana-design (Team:Kibana-Design)

@kertal
Copy link
Member

kertal commented Oct 15, 2021

This can't be changed in Kibana , it should be moved to EUI

@1Copenut 1Copenut transferred this issue from elastic/kibana Oct 15, 2021
@1Copenut
Copy link
Contributor Author

Thanks for the followup @kertal. I moved this to EUI and have re-tagged it.

@1Copenut 1Copenut added this to Needs triage/design in Accessibility via automation Oct 15, 2021
@cchaos
Copy link
Contributor

cchaos commented Oct 25, 2021

@1Copenut The EuiModal component doesn't create the heading level directly. All of our docs examples shows wrapping the title text in an <h1> like so:

<EuiModalHeaderTitle>
  <h1>Modal title</h1>
</EuiModalHeaderTitle>

The statement that was made:

This can't be changed in Kibana , it should be moved to EUI

Is actually false, as all they need to do is wrap their title text in a HTML heading.

@cchaos
Copy link
Contributor

cchaos commented Oct 25, 2021

If we need to, in the EUI docs, we can add a strong callout identifying this aspect. But it would be a truly harsh update to those who were copying the examples properly to have to unwrap all their text.

@cchaos
Copy link
Contributor

cchaos commented Oct 25, 2021

I spoke somewhat too harshly too soon, it does look like our EuiConfirmModal specifically doesn't wrap the title if it's simply provided as a string. I've updated the title of the issue to reflect this.

Screen Shot 2021-10-25 at 15 22 06 PM

@cchaos cchaos changed the title [COGNITION]: Text that appears like a heading must be marked up as a heading [EuiConfirmModal] Title should be wrapped in an HTML heading if provided as a string Oct 25, 2021
@1Copenut 1Copenut moved this from Needs triage/design to In progress in Accessibility Dec 20, 2021
@1Copenut 1Copenut self-assigned this Dec 20, 2021
Accessibility automation moved this from In progress to Closed Dec 21, 2021
@1Copenut
Copy link
Contributor Author

1Copenut commented Jan 10, 2022

@kertal I wanted to follow up on this issue. I made a change to our EUI component to wrap strings in an H1, but I'm still seeing DIV containers in Kibana 8 on the main branch locally. If consumers are passing in a legitimate React element, the component will not override it with a heading. Is there additional action Kibana teams can make, or does this issue have a longer tail than we first thought?

@cee-chen
Copy link
Member

I made a change to our EUI component to wrap strings in an H1, but I'm still seeing DIV containers in Kibana 8 on the main branch locally

@1Copenut your fix isn't yet in Kibana - the upgrade PR is still open (elastic/kibana#122386). I would wait until that PR lands to check the main branch, or check that branch directly

@1Copenut
Copy link
Contributor Author

Thank you @constancecchen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Accessibility
  
Closed
Kibana Upstream A11y
Awaiting triage
Development

Successfully merging a pull request may close this issue.

5 participants