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

feat: Card component #8154

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

feat: Card component #8154

wants to merge 9 commits into from

Conversation

jouni
Copy link
Member

@jouni jouni commented Nov 14, 2024

Documentation is lacking CSS custom props/styling. Not sure if anything else. Oh, tests, of course 😄

Light Dark
localhost_8000_dev_card html (4) localhost_8000_dev_card html (5)

dev/card.html Outdated
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I spend time reducing the duplication?

Copy link

sonarcloud bot commented Nov 15, 2024


/** @protected */
render() {
return html`<slot></slot>`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks kinda basic 🙂 no support for header, content and footer like dialog? Unify API and usage for developers before they have to do it again and again

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Yeah, we are very much aware of this. We are just not yet sure how we want to implement all of that and what kind of API would be suitable, and want to move quite slowly in the beginning before locking in on anything more specific.

We just spent some hours talking with @rolfsmeds about this, and how card relates to the proposed "item" component, and the other "header-content-footer" component we’re thinking about, how all those fit together ("Are they the same component or smaller pieces you need to compose together to get what you need? If they’re not the same component, how do we share the styles and layouts between them? How do we name all of them? Could they just be utility classes? How does all of that affect discoverability? What is even knowledge?").

Copy link
Member Author

@jouni jouni Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And as you mentioned Dialog, we also touched on that, and will likely want to refactor it to use the "header-content-footer" layout we’re pondering (eventually, whenever that might happen).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for publishing your thoughts 🙂 I was sadly kinda expecting it.. especially combined with the recent discussion you linked.

My concern comes from the way of using / adapting to a new component: what is the benefit for me as developer? If it's just a "wrapper" with no real functionally (header footer and so on) it "feels" unfinished - which lefts me with two options: don't use it.. or use it, extend it and customize it. Often I'm seeing myself doing the second option because - if available - I want to use the official component as base. Which in turn creates problems when "unfinished" components gets later enriched because this can clash with my already enhanced version of it. That's why I mentioned Dialog where exactly that happened when I migrated to your provided header and footer slots. IMHO those "basic" constructs should have been available at the beginning... Therefore I would request them here as well.. even tho it could push this component one minor release in the future when some designs are needed.. it would highly benefit customer before they have to work three times implementing / using it.

  1. Ditch their implementation for yours
  2. Customize your implementation because it's not enough
  3. After your implementation is enhanced.. get rid of customizing

While I agree that it your be beneficial to have a shared item base for dialog, card and what not for.. it feels kinda overenginered IMHO.. yes they could share styles.. but I personally would be against it. This could easily create problems where you wanna style "vaadin-item" however since vaadin-dialog is also using it.. those customizing might leak or developers are not aware of the consequences.. especially when you squeeze in css variables.

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

Successfully merging this pull request may close these issues.

3 participants