Semantic-Org / Semantic-UI-React

The official Semantic-UI-React integration
https://react.semantic-ui.com
MIT License
13.22k stars 4.05k forks source link

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

Closed DEfusion closed 8 years ago

DEfusion commented 8 years ago

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

Failed prop type: Invalid proppanels[0].contentof typeobjectsupplied toAccordion, expectedstring.

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 commented 8 years ago

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 commented 8 years ago

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.