[EuiErrorBoundary] Use EuiCodeBlock to render error #5359
Conversation
| </div> | ||
| <EuiCodeBlock> | ||
| <EuiTitle size="xs"> | ||
| <p>Error</p> |
There was a problem hiding this comment.
💯 I think changing this to a <p> makes more sense from an accessibility point of view- feel free to chime in if I'm missing something @1Copenut
There was a problem hiding this comment.
@constancecchen I agree, paragraphs are more meaningful than divs here. It's permissible according to the spec, but this is a step up.
There was a problem hiding this comment.
For more context, this was actually an <h1> before. I'm not sure how our automated a11y tests didn't catch this page (it's not excluded). But I didn't think that top level heading was appropriate and usually just opt for paragraphs if we can't know for sure where this will lie in DOM hierarchy.
There was a problem hiding this comment.
I'm guessing the page was either excluded or we're not doing best-practices checks. Either way, this works better and is semantically meaningful. I'll change my comment to approved.
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5359/ |
|
Thank you for being so quick to address the issue @cchaos! 🙌 |
Fixes #5358
Before the component was just rendering a bare
<pre>element, but now the error gets wrapped in our EuiCodeBlock which handles the overflow scrolling and gets rid of some unnecessary custom styles and selectors.Before
After
Checklist
[ ] Props have proper autodocs and playground toggles[ ] Added documentation[ ] Checked Code Sandbox works for any docs examples[ ] Added or updated jest and cypress tests[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes