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

Accordion component #157

Merged
merged 35 commits into from May 21, 2019

Conversation

Projects
None yet
3 participants
@RobertBolender
Copy link
Collaborator

commented May 1, 2019

Ready for review.

Notes to myself / TODO list:

  • Basic keyboard navigation

  • Basic collapsing behavior

  • Reduced padding on mobile, according to the spec.

  • Arrow key navigation should only be available when a header is selected. All form elements within the body should be in the normal tab order, and while focus is within the body there is no special focus handling.
    § Keyboard Interaction

  • Headers should receive focus when clicked.

  • Visually persistent elements that appear adjacent to the header should not be descendants within the DOM. My current approach is to use CSS Grid to position the element visually, but if the element is interactive it will still be within the document's tab order.
    I think that ARIA might be happier if the header element was focusable and the adjacent indicator element was not focusable. We could still have visual styling that makes it look like the indicator was the focused element.
    § WAI-ARIA Roles, States, and Properties

  • I've decided against using a PropTypes.node prop for customIndicator. For now I'm also not going to add a "left" or "right" indicatorPosition prop.

  • I'm going to use a render prop for the customIndicator, passing it an isExpanded prop.

  • Add the appropriate aria-level attribute. Handled by HTML heading elements.

  • Added a PropTypes.node prop for subtitle.

  • Added the appropriate aria-controls attribute.

@RobertBolender RobertBolender added the WIP label May 1, 2019

@RobertBolender RobertBolender added enhancement and removed WIP labels May 14, 2019

@RobertBolender RobertBolender force-pushed the RobertBolender:accordion-component branch from bf1a2c8 to 2bef118 May 14, 2019

@RobertBolender RobertBolender marked this pull request as ready for review May 14, 2019

@RobertBolender

This comment has been minimized.

Copy link
Collaborator Author

commented May 14, 2019

I want to run this by design real quick to make sure I've covered everything in the spec, but the code is ready for review.

Edit: I have submitted this for UI review.
https://beta.faithlife.com/messages/24703?threadId=647805

@bryanrsmith
Copy link
Collaborator

left a comment

I didn't run it to review UI, but looks pretty good to me!

Show resolved Hide resolved components/accordion/accordion-header.jsx Outdated
Show resolved Hide resolved components/accordion/accordion-header.jsx Outdated
Show resolved Hide resolved components/accordion/accordion-header.jsx
Show resolved Hide resolved components/accordion/accordion-header.jsx Outdated
Show resolved Hide resolved components/accordion/accordion-header.jsx Outdated
Show resolved Hide resolved components/accordion/accordion-item.jsx Outdated
Show resolved Hide resolved components/accordion/accordion-item.jsx
Show resolved Hide resolved components/accordion/accordion-util.js
Show resolved Hide resolved components/main.js Outdated
Show resolved Hide resolved catalog/accordion/variations.md Outdated

@korbinancell korbinancell merged commit 0f53982 into Faithlife:master May 21, 2019

1 check passed

netlify/faithlife-styled-ui/deploy-preview Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.