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/AccordionPanel Review Notes #960

Open
9 tasks
Darper opened this issue Oct 8, 2018 · 1 comment
Open
9 tasks

Accordion/AccordionPanel Review Notes #960

Darper opened this issue Oct 8, 2018 · 1 comment

Comments

@Darper
Copy link
Contributor

Darper commented Oct 8, 2018

Accordion.js

  • Line 57: Add comment to inform that we are checking for a string in case the user uses an HTML element instead of a React Node, in which case boolean attributes aren't allowed
  • Lines 115-116: Remove the titlePosition prop and allow user to set positioning/style by passing through a Node instead of a String to the title prop.
  • Line 118: Remove header-inactive class and make that the default styling. Only overwrite when active.
  • Lines 130-131: Do we need these?
  • Line 134: Remove unnecessary wrapper div. Replace with React.Fragment if wrapper is needed.
  • Line 168: Wrap message in <Text> component.
  • Line 197: Allow expandAll prop to be either boolean or string. If string, use that value instead of messages.expandAll on line 168.

AccordionPanel.js

  • Line 17: Delete panelStyles variable and just set styleName=cx("accordionPanel") on div.
  • Line 49: Set prop type for title to PropTypes.node to allow user to pass in any element. Check styling to make sure end user has full control over the header section when adding a title element.
@Darper
Copy link
Contributor Author

Darper commented Nov 5, 2018

Change expanded to active

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant