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

Fix footer actions on content grid not aligning to the base of the container #13445

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

rossbearman
Copy link
Contributor

When a table is displayed as a grid using contentGrid(), if the heights of each row box differs, the footer actions will stick to the bottom of the content, rather than sitting flush with the bottom of the box.

A couple of people have encountered and reported this, and this fix has worked in those cases, but this PR has not been thoroughly tested, though it is a small change.

Visual Difference

Before:
image

After:
image

Functional changes

  • Conditionally switch to using flex h-full items-stretch on content grid rows in place of flex items-center

@zepfietje zepfietje self-assigned this Jul 1, 2024
@zepfietje zepfietje added bug Something isn't working ui labels Jul 1, 2024
@zepfietje zepfietje added this to the v3 milestone Jul 1, 2024
@zepfietje
Copy link
Member

I'm going to hold off on merging this PR, because I'm not sure if having the actions aligned at the bottom is desired for every use case.

@zepfietje zepfietje closed this Jul 19, 2024
@rossbearman
Copy link
Contributor Author

Would it make sense to have this available as an option on the content grid instead?

@zepfietje
Copy link
Member

Ideally there'd just be an actions component you could use to create the exact layout you want. Not sure if we're open to an API to configure the current actions position. What do you think, @danharrin?

@danharrin
Copy link
Member

You'd need to be able to configure the parent container to align the actions to the bottom, so I don't think thats right

@zepfietje
Copy link
Member

From your comment it's unclear to me what direction we want to take, @danharrin.
Care to clarify? 🙂

@danharrin
Copy link
Member

Personally I can't think of a reason why this PR cannot be merged as-is

@zepfietje zepfietje reopened this Jul 25, 2024
@zepfietje
Copy link
Member

Alright, let's reconsider this then.

Copy link
Member

@zepfietje zepfietje left a comment

Choose a reason for hiding this comment

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

Could you please share screenshots of before and after for this change with bulk actions enabled, @rossbearman? Need to make sure this doesn't break any of the combinations (reorder indicator or bulk select checkbox).

@@ -542,7 +542,8 @@ class="col-span-full"

<div
@class([
'flex items-center',
'flex items-center' => ! $contentGrid,
Copy link
Member

Choose a reason for hiding this comment

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

Please extract the common classes.

Copy link
Member

Choose a reason for hiding this comment

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

Are you able to clean this up, @rossbearman? 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's on my list to get to, but I'm snowed under with work currently.

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

Successfully merging this pull request may close these issues.

3 participants