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 different element type for EuiCard description #1015

Closed
pugnascotia opened this issue Jul 16, 2018 · 5 comments
Closed

Allow different element type for EuiCard description #1015

pugnascotia opened this issue Jul 16, 2018 · 5 comments
Assignees

Comments

@pugnascotia
Copy link
Contributor

EuiCard wraps the description in <p> tags. It would be useful to be able to change this to e.g. <div> to avoid warnings about invalid DOM nesting, such as when putting a list in the description.

@chandlerprall
Copy link
Contributor

We've employed the pattern of including a component prop in these cases where the consumer can specify what tag to use. c.f. #1011

@cchaos
Copy link
Contributor

cchaos commented Jul 16, 2018

@chandlerprall That's when it's absolutely necessary to wrap the children in a parent element. For this type of case, I think we just need to allow a string or node, and if it's a string we wrap it in a paragraph tag. We'll want to do the same for the title. #979

@cchaos cchaos self-assigned this Jul 19, 2018
@cchaos
Copy link
Contributor

cchaos commented Jul 19, 2018

I gave this some more thought and EuiCard was built so that we'd have consistency of how certain content is laid out. That certain content is a Panel with a title, description (paragraph) and icon or image. It's almost more of a pattern than a hard-and-fast component and is built using all EUI components.

I looked at the implementation in cloud (screenshot from the mentioned issue) and honestly, those are so far different from the basic card concept, that it can and should just be a custom component built with EuiPanel, EuiTitle, and EuiText.

In the end, I'm going to leave the description as-is and close this ticket in favor of consumers creating their own "card"s when their content doesn't match the default EuiCard.

@cchaos cchaos closed this as completed Jul 19, 2018
@pugnascotia
Copy link
Contributor Author

I understand the reasons for closing the issue, but I'd ask that you reconsider. Cloud UI was in fact using its own card component, but I changed it to use EUI's as it correctly positions the footer icons. If you ignore the content of cards, it's a component with a title, some content, a footer, and possibly a badge - just what Cloud needs. We would be copying the source verbatim but changing p to div, just to silence a warning. I would have no objections to "forking" the component in the event that EUI needs to take it in a different direction.

@bevacqua
Copy link
Contributor

+1, forcing the kind of children is not something that's done elsewhere in EUI, so I'd favor consistency. Originally CJ had string prop types for many things that were meant to be any kind of children, so that might be the source of the dissonance here, but most of these where changed to a node proptype so we can do whatever we see fit. Prescribing too much semantics (and styling based around that) is not a great long term approach imo

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

No branches or pull requests

4 participants