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

Allow for caption to be specified for Table components #463

Closed
chawes13 opened this issue Jan 6, 2021 · 5 comments · Fixed by #481
Closed

Allow for caption to be specified for Table components #463

chawes13 opened this issue Jan 6, 2021 · 5 comments · Fixed by #481
Assignees
Milestone

Comments

@chawes13
Copy link
Contributor

chawes13 commented Jan 6, 2021

We should be able to specify the text or component to use as a <caption> element in our Table components. We could leverage a similar pattern as established for label / labelComponent in LabeledField, i.e., accept a caption string prop and a captionComponent prop that defaults to ({ caption }) => <caption>{caption}</caption>.

This is related to research by @mwislek in addressing #324

@dpikt
Copy link
Contributor

dpikt commented Jan 7, 2021

👍good callout. I think we could just make this a node prop that will be inserted into the tag since it wouldn't need access to any "internal" props/state (I think):

caption="My Table"

...

caption={<p>My<strong>important</strong>table</p>}

@chawes13
Copy link
Contributor Author

chawes13 commented Jan 7, 2021

Is there a pattern for accepting Node|String? Passing in text seems like the primary use case, and caption={<>My Table</>} feels a little clunky. Or since this is a node, I suppose the following would be equivalent?

caption="My Table"
<caption>{caption}</caption>

caption=<p>My <strong>Table</strong></p>
<caption>{caption}</caption>

@dpikt
Copy link
Contributor

dpikt commented Jan 7, 2021

A string is a node, so it would work that way by default 🙂 My example above was an "inclusive or" of the ways it could be used.

@dpikt
Copy link
Contributor

dpikt commented Jan 7, 2021

To be clear, I mean node as in PropTypes.node, not a DOM node.

@chawes13
Copy link
Contributor Author

chawes13 commented Jan 7, 2021

Awesome, thanks for the clarification!

@chawes13 chawes13 added this to the v6 milestone Aug 31, 2021
@chawes13 chawes13 linked a pull request Nov 22, 2021 that will close this issue
5 tasks
@chawes13 chawes13 linked a pull request Jun 16, 2022 that will close this issue
5 tasks
@chawes13 chawes13 mentioned this issue Jun 23, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants