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

[Feature] Expander improvements #414

Merged
merged 16 commits into from
Dec 7, 2020
Merged

Conversation

aappoalander
Copy link
Contributor

@aappoalander aappoalander commented Nov 30, 2020

Description

This PR introduces composition pattern for Expander.

  • Expander content is now described using ExpanderTitle and ExpanderContent components that can contain ReactNodes.
  • Expander groups can now contain ReactNodes in addition to expanders

In addition, multiple internal improvements are included:

Related Issue

In addition multiple bugfixes are included:

  • Content is no longer focusable/focused after closing an expander.
  • Support for custom open/close all button is removed
  • Aria attributes to support screen readers with open/close all button added
  • Open/Close button stays in sync

Motivation and Context

Expander and ExpanderGroup implementation was no longer up-to-date with design specification and had quite a few issues. It was better to do a larger refactor rather than trying to address all issues with small fixes.

How Has This Been Tested?

Tested with TBA (CRA-TS, SSR, Styleguidist, DS-Site) using supported browsers and devices

Screenshots (if appropriate):

  • No changes to visuals

Release notes

  • Breaking changes: Complete API redesign and switch to content composition pattern. See [Feature] Expander improvements #414
    • Expander API
      • Removed: titleTag, title, titleProps, contentProps and internally used expanderGroup and index
      • Renamed: onClick => onOpenChange
      • Added: id
    • ExpanderGroup API
      • Static version Expander.group is no longer available. Use ExpanderGroup instead
      • Removed: onClickAll
      • Renamed: OpenAll => OpenAllText, CloseAll => CloseAllText
      • Added: AriaCloseAllText, AriaCloseAllText, toggleAllButtonProps, className

Screenshot 2020-11-30 at 16 47 00

@aappoalander aappoalander added bug Something isn't working enhancement New feature or request Accessibility labels Nov 30, 2020
@aappoalander aappoalander self-assigned this Nov 30, 2020
@ketsappi
Copy link
Contributor

ketsappi commented Dec 2, 2020

As the static version, Expander.group is no longer available because the API changes it might be good to mention that in release notes & PR description.

@aappoalander aappoalander added the DX Refactoring for maintainability label Dec 3, 2020
jest.setup.js Outdated Show resolved Hide resolved
@aappoalander aappoalander merged commit 177816a into develop Dec 7, 2020
@aappoalander aappoalander deleted the feature/move-expander-to-core branch December 30, 2020 11:51
@aappoalander aappoalander mentioned this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility bug Something isn't working DX Refactoring for maintainability enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants