feat(SourcesCardBase): Expose just card portion of SourcesCard#674
feat(SourcesCardBase): Expose just card portion of SourcesCard#674rebeccaalpert merged 2 commits intopatternfly:mainfrom
Conversation
|
Preview: https://chatbot-pr-chatbot-674.surge.sh A11y report: https://chatbot-pr-chatbot-674-a11y.surge.sh |
Allows consumers to custom-roll their own layouts using preexisting component.
rebeccaalpert
force-pushed
the
sources-card-export
branch
from
d1f85e4 to
a9e1876
Compare
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Overall I think things look fine, I may just be a bit confused as to the use case separating things out in this way resolves.
With the react repo we'll use "ComponentBase" within a component file, then export "Component", rather than having 2 separate files usually (some deviations from this, though, like AlertGroupInline I think being one of them, though we don't export them for public use). I would also think about how we'd differentiate these 2 components since they're so similarly named.
- Would we be able to add an example that shows how this update is intended to work?
- If we need to export both of these to be used by consumers, maybe we can consider different names for the two?
- Does this tie into the use case of instead of paginating and showing only 1 card at a time, there was the design where multiple cards were shown in an overflow sort of container similar to the PF Nav component?
| ); | ||
| }; | ||
| }: SourcesCardProps) => ( | ||
| <div className="pf-chatbot__source"> |
There was a problem hiding this comment.
Do we want to nest SourcesCardBase which also has this div wrapper inside of here, or should only 1 have this div wrapper?
There was a problem hiding this comment.
Gave it a unique name!
|
We have a separate issue for an example! It was definitely for the use case of a DIY overflow container. We can't change the name of the original component without breaking folks, but I don't really care if use a different name for the actual card piece. |
rebeccaalpert
force-pushed
the
sources-card-export
branch
from
81f8d92 to
88ba220
Compare
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Not entirely sure what an alternative name for SourcesCardBase could be, so I'm willing to forgo addressing that part of feedback for now. Only idea would be something like "SingleSourceCard" but idk if that'd be super fitting.
I think as long as part of a followup to add an example showcasing the differences between these 2 components we can ensure documentation is clear on it, it may not be a huge thing.
|
Kayla has the extra fancy Sources card navigation design on her docket for Q4 fwiw! |
|
🎉 This PR is included in version 6.4.0-prerelease.24 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…rnfly#674) Allows consumers to custom-roll their own layouts using preexisting component.
3 participants
Allows consumers to custom-roll their own layouts using preexisting component.