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

card-header and card-footer need radius overrides when bg-color-X set #2562

Closed
niktek opened this issue Mar 21, 2024 · 3 comments
Closed

card-header and card-footer need radius overrides when bg-color-X set #2562

niktek opened this issue Mar 21, 2024 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@niktek
Copy link
Contributor

niktek commented Mar 21, 2024

Current Behavior

When setting a bg color on a card-header or card-footer it will overpaint the containing card and its border-radius setting. I think if the user is opting into a card then it should carry through the expected border-radius to the child elements. The following uses tokens to re-apply the border radius on the card-header, but removing them will make it appear like card-footer.

<div class="p-5">
	<div class="card my-10 ">
		<header class="card-header bg-primary-500 rounded-tl-container-token rounded-tr-container-token">Actions</header>
		<section class="p-4">
			<button class="btn variant-filled">Save</button>
		</section>
		<footer class="card-footer bg-primary-500">(footer)</footer>
	</div>
</div>

It also raises a question of whether we should also extend the rounded tokens to match the options TW provides as we are missing tokens for (would we still keep container in there?):

rounded-s-container-token
rounded-e-container-token
rounded-t-container-token
rounded-r-container-token
rounded-b-container-token
rounded-l-container-token
rounded-ss-container-token
rounded-se-container-token
rounded-ee-container-token
rounded-es-container-token

Expected Behavior

No response

Steps To Reproduce

No response

Link to Reproduction / Stackblitz

No response

More Information

No response

@niktek niktek added the bug Something isn't working label Mar 21, 2024
@endigo9740
Copy link
Contributor

endigo9740 commented Mar 21, 2024

@niktek I think this would be easier to understand with a screenshot.

That said, rounded styles now extend Tailwind's default border radius styles in v3. So this issue would be limited to v2. Header/Footer styles are also gone in v3.

@endigo9740 endigo9740 modified the milestones: v3.0 (Next), v2.0 Mar 21, 2024
@niktek
Copy link
Contributor Author

niktek commented Mar 22, 2024

image
<div class="p-5">
	<div class="card">
		<header class="card-header">Actions</header>
		<section class="p-4">
			<button class="btn variant-filled">Save</button>
		</section>
		<footer class="card-footer">(footer)</footer>
	</div>
</div>

<div class="p-5">
	<div class="card">
		<header class="card-header bg-primary-500">Actions</header>
		<section class="p-4">
			<button class="btn variant-filled">Save</button>
		</section>
		<footer class="card-footer bg-primary-500">(footer)</footer>
	</div>
</div>

<div class="p-5">
	<div class="card">
		<header class="card-header bg-primary-500 rounded-tl-container-token rounded-tr-container-token">Actions</header>
		<section class="p-4">
			<button class="btn variant-filled">Save</button>
		</section>
		<footer class="card-footer bg-primary-500 rounded-bl-container-token rounded-br-container-token">(footer)</footer>
	</div>
</div>

It's not a show stopper at all, but it also doesn't feel right either.

@endigo9740
Copy link
Contributor

endigo9740 commented Apr 5, 2024

@niktek sorry for the delay on this one. Now that I've had a chance to read through this with a clear mind I see what you mean. The goal with the v2 implementation was to keep a unified spacing around the edge of the screen, as well as between sections.

For example, all yellow lines shown should have uniform spacing:

Screenshot 2024-04-05 at 4 08 55 PM

Unfortunately, if you double up the spacing by adding padding within the children, this will lead to a poor result where space appears doubled (or tripled) between the children. This can result in styles that feel very uneven:

Screenshot 2024-04-05 at 4 19 43 PM

That said, if I was to try and handle this sort of layout today, what I'd instead opt for is custom styles like this. This approach should work in Skeleton v2 or v3:

<!-- Surrounding Gap -->
<div class="card p-4 space-y-4">
	<header class="bg-primary-500">Header</header>
	<section>Content</section>
	<footer class="bg-primary-500">Footer</footer>
</div>

<!-- Flush -->
<div class="card">
	<header class="p-4 bg-primary-500">Header</header>
	<section class="p-4">Content</section>
	<footer class="p-4 bg-primary-500">Footer</footer>
</div>

Given the header/footer concepts won't be carrying forward to Skeleton v3, I'd recommend following the pattern above for now. I don't think there's a sane way we can handle every use case folks might in v2, so we'll call this a wash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants