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

Clarify if/how to use components in accordion panels? #559

Closed
DEfusion opened this issue Sep 28, 2016 · 2 comments
Closed

Clarify if/how to use components in accordion panels? #559

DEfusion opened this issue Sep 28, 2016 · 2 comments
Labels

Comments

@DEfusion
Copy link

I'm trying to use components within my accordion panels and while everything works I get this warning:

Failed prop type: Invalid prop panels[0].contentof typeobjectsupplied toAccordion, expected string.

Which I guess is because of the customPropTypes definition for panels.

Here's my panels method:

return this.years().map((data) => {
    return { title: data.year, content: <div>...</div> };
});

I've also tried building an array like below, but I don't get the dropdown Icon as renderPanels inserts that not AccordionTitle

years.forEach((data) => {
      panels.push(<Accordion.Title>{data.year}</Accordion.Title>);
      panels.push(<Accordion.Content>...</Accordion.Content>);
    });

What's the correct way to do this and should the documentation be updated to clarify the best way?

@levithomason
Copy link
Member

The Accordion API is on my list of things to review. I'd like to improve the array shorthand. We're currently midway through improving our shorthand for single value props. Array value props will be next. I also plan to add a doc page dedicated to shorthand. Shorthand is powerful, but will be so much more powerful once we update handling of arrays.

Here's the scoop on the current state of things.

Prop Warning

When using the array shorthand, the object values are limited to strings at present. Though any renderable value (node) will work, it is technically not supported. There are technical reasons for this that span across all of our components. As we continue to develop our shorthand capabilities, this may change. Though for now, strings are the safe bet for accordion panel shorthand.

http://technologyadvice.github.io/stardust/modules/accordion#panels-prop

Missing Icon

When defining children, you need to define all the markup yourself, including Icon. Your array would work if you simply add a dropdown icon into the Title.

http://technologyadvice.github.io/stardust/modules/accordion#accordion

<Accordion>
  <Accordion.Title>
    <Icon name='dropdown' />
    What is a dog?
  </Accordion.Title>
  <Accordion.Content>
    <p>
      A dog is a type of domesticated animal. Known for its loyalty and faithfulness,
      {' '}it can be found as a welcome guest in many households across the world.
    </p>
  </Accordion.Content>
</Accordion>

Improvements

Again, we're in development and I expect the Accordion API to be improved considerably. Though, it is lower on the priority list for me personally. I'd gladly help spec out those improvements if someone wanted to start a PR. Cheers!

@DEfusion
Copy link
Author

Thanks for the detailed reply, I'd volunteer to do some work on the Accordion API but I'm relatively new to React so I'm not sure how much help I can give even if there were a spec for the API changes. I have started looking at why defaultActiveIndex didn't seem to work for me though.

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

No branches or pull requests

2 participants