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.

Sign up for GitHub

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

Document EuiAccordion disabled state #6088

Closed
wants to merge 1 commit into from
Closed

Document EuiAccordion disabled state #6088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src-docs/src/views/accordion/accordion_disabled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';

import { EuiAccordion, EuiFieldText, EuiFormRow, EuiSpacer } from '../../../../src/components';
import { useGeneratedHtmlId } from '../../../../src/services';

export default () => {
const disabledAccordionId = useGeneratedHtmlId({ prefix: 'disabledAccordion' });

return (
<div>
<EuiAccordion
id={disabledAccordionId}
buttonContent="Security settings"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see this buttonContent prop expanded to include a conditional EuiScreenReaderOnly component that further explains the disabled state. If I'm using a screen reader and listening to the Form Controls menu, I'm not going to have any of the error state context. The only thing I will hear is the label "Security settings, dimmed button."

This signals the button is there, but not why it's dimmed. A better message might be "Security settings has errors in expanded content. dimmed button." This isn't exact wording, but a for instance.

The EUI docs show buttonContent as a React Node, so you should be able to do something like the following snippet, assuming a new prop isDisabled:

+ const disabledButtonContent =
+  <>
+    <span>Security settings</span>
+    <EuiScreenReaderOnly>
+      <span>{' '}has errors in expanded content.</span>
+    </EuiScreenReaderOnly>
+  </>;

- buttonContent="Security settings"
+ buttonContent={isDisabled ? disabledButtonContent : "Security settings"}

Screen Shot 2022-07-28 at 12 20 09 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

@1Copenut If we were to build isDisabled directly into EuiAccordion, do you have any other advice we should be requiring of the consumer for the disabled state?

Copy link
Contributor

Choose a reason for hiding this comment

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

Off the top of my head the only other recommendation would be to only use isDisabled with Accordions that render a button on the page. Having a disabled attribute on a fieldset will thrown an error in axe (linter, unit tests, et al.)

arrowProps={{ disabled: true }}
buttonProps={{ disabled: true }}
initialIsOpen={true}
>
<EuiSpacer size="s" />
<EuiFormRow
label="Password"
isInvalid={true}
error={[
"You must enter your password to save these changes.",
]}
>
<EuiFieldText name="text" isInvalid={true} />
</EuiFormRow>
</EuiAccordion>
</div>
);
};
38 changes: 38 additions & 0 deletions src-docs/src/views/accordion/accordion_example.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ const accordionIsLoadingSnippet = [
import AccordionButtonElement from './accordion_buttonElement';
const accordionButtonElementSource = require('!!raw-loader!./accordion_buttonElement');

import AccordionDisabled from './accordion_disabled';
const accordionDisabledSource = require('!!raw-loader!./accordion_disabled');

export const AccordionExample = {
title: 'Accordion',
intro: (
Expand Down Expand Up @@ -420,5 +423,40 @@ export const AccordionExample = {
<!-- Content to show when expanded -->
</EuiAccordion>`,
},
{
title: 'Disabled',
source: [
{
type: GuideSectionTypes.JS,
code: accordionDisabledSource,
},
],
text: (
<>
<p>
In some cases you might want to prevent the user from closing the
<strong>EuiAccordion</strong>. For example, if a form field is
displaying an error, opening the accordion and preventing its
closure until the error has been addressed will help the user
find and fix the error.
</p>
<p>
The <EuiCode>arrowProps</EuiCode>, <EuiCode>buttonProps</EuiCode>,
and <EuiCode>initialIsOpen</EuiCode> props can be used together
to achieve this state.
</p>
</>
),
demo: <AccordionDisabled />,
playground: accordionConfig,
props: { EuiAccordion },
snippet: `<EuiAccordion
Comment on lines +451 to +453
Copy link
Contributor Author

@cjcenizal cjcenizal Jul 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'd also appreciate guidance on what to do with these properties ln 451-453. 🙇

id={accordionId1}
buttonContent="Clickable title"
>
<!-- Content to show when expanded -->
</EuiAccordion>
`,
},
],
};