Combine chip and chipGroup docs #8968
Conversation
|
Preview: https://patternfly-react-pr-8968.surge.sh A11y report: https://patternfly-react-pr-8968-a11y.surge.sh |
edonehoo
left a comment
edonehoo
left a comment
There was a problem hiding this comment.
Initial review re: headings. Open to feedback/questions! I can also edit/add documentation for each example in a followup review since there are only a couple.
| --- | ||
|
|
||
| ## Examples | ||
| ## Chip examples |
There was a problem hiding this comment.
| ## Chip examples | |
| ## Examples |
Since the rest of our component docs call this section "Examples", I think it would be best to remain aligned with that - even if we're combining a couple of former components. This also aligns with what we do with toolbar, for example.
There was a problem hiding this comment.
@edonehoo The reason we did this was to differentiate between the two sets of examples in the page - so we have Chip examples and Chip group examples. We used that same pattern when combining Alert and Alert group: https://patternfly-react-pr-8968.surge.sh/components/alert Or are you saying that we don't need a second h2 heading for Chip group examples?
There was a problem hiding this comment.
@mcarrano Ah, I haven't seen the Alert PR yet. Thanks for linking -- it's helpful to see an example with more "sub" examples in each section. Toolbar is kind of a different case, so it's not the best comparison.
I was saying that we don't need a second h2 heading to split/differentiate the "base" vs "group" examples, because it felt like we were still keeping the two original components separate, just on a new page. But, maybe keeping them split makes sense since they are different React components?
But this is kind of splitting hairs and I don't have a huge preference either way - so long as we are consistent and make sure that the "blurb" description is updated to cover both example groups. If we do decide to keep them split I would change the headings to "Examples" and "Chip group examples". I still feel inclined to keep the first heading consistent with the rest of our components.
There was a problem hiding this comment.
I do feel that Chip/ChipGroup is different from the Alerts. With the Alert and AlertGroup, both components had variants and variant examples and we need a way to distinguish the two. With the Chip and chip group there is really no overlap in examples.
Here is how the Chip Group documentation looked before we split it it from Chip before. I think it was fine as it was without having to create sub categories for Chip vs Chip group.
https://www.patternfly.org/2021.15/components/chip-group
| ``` | ||
|
|
||
| ## Chip group examples | ||
| A chip group is a collection of chips that can be grouped by category and used to represent one or more values assigned to a single attribute. When the value of `numChips` is exceeded, additional chips will be hidden using an overflow chip. |
There was a problem hiding this comment.
| A chip group is a collection of chips that can be grouped by category and used to represent one or more values assigned to a single attribute. When the value of `numChips` is exceeded, additional chips will be hidden using an overflow chip. |
Move this to the simple inline chip groups example to better indicate that all of the examples are part of the "chips" component?
Co-authored-by: edonehoo <105813956+edonehoo@users.noreply.github.com>
Co-authored-by: edonehoo <105813956+edonehoo@users.noreply.github.com>
Co-authored-by: edonehoo <105813956+edonehoo@users.noreply.github.com>
nicolethoen
left a comment
nicolethoen
left a comment
There was a problem hiding this comment.
This LGTM. I'm okay with changing a few heading titles.
I don't have a strong opinion one way or the other whether we still want to partition the Chip and Chip group examples with differentiated titles.
| --- | ||
|
|
||
| ## Examples | ||
| ## Chip examples |
There was a problem hiding this comment.
I do feel that Chip/ChipGroup is different from the Alerts. With the Alert and AlertGroup, both components had variants and variant examples and we need a way to distinguish the two. With the Chip and chip group there is really no overlap in examples.
Here is how the Chip Group documentation looked before we split it it from Chip before. I think it was fine as it was without having to create sub categories for Chip vs Chip group.
https://www.patternfly.org/2021.15/components/chip-group
edonehoo
left a comment
edonehoo
left a comment
There was a problem hiding this comment.
LGTM! I'll plan to add content to the undocumented examples in a follow up PR.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
5 participants
Combines docs for Chip and ChipGroup into a single page. Note that I added some documentation to the example since there was none previously.
What: Closes #8744