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

CardComponent: Has a Header Slot #2114

Merged
merged 3 commits into from
Jan 21, 2024
Merged

Conversation

zspencer
Copy link
Member

This is perhaps unnecessary; since <header> tags are easy to write; but maybe it's actually useful?

@zspencer zspencer requested review from a team January 13, 2024 04:11
- #2110

This is perhaps unnecessary; since `<header>` tags are easy to write;
but maybe it's actually useful?
OK so TIL about [ViewComponent Lambda
Slots](https://viewcomponent.org/guide/slots.html#lambda-slots), which
seem like a decent way for paramaterizing headers and footers.
overridden.

There are variants for the Header and Footer, which seems nice

It could be good to update the ComponentPreview as well...
DEFAULT_FOOTER = "bg-slate-50 p-2 sm:p-4"
FOOTER_VARIANTS = {
default: DEFAULT_FOOTER,
action_bar: [DEFAULT_FOOTER, "flex flex-row justify-between"].join(" ")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: create a separate ActionCardComponent -- with duplicate styling/code as necessary. I tend to dislike parameterizing layout or style unless it's related to component state. But that's because I've seen this type of parameterization grow unwieldy when a single component tries to handle too many use cases. We're not there yet, of course.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, I was thinking if we got to 3+ variants I would want to start pulling out the more special-purpose ones; and I feel like an ActionCardComponent is a pretty good next step when that happens!

@zspencer zspencer merged commit 85ffdf2 into main Jan 21, 2024
4 checks passed
@zspencer zspencer deleted the card_component/has-a-header branch January 21, 2024 03:30
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.

2 participants