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

Put bulk actions above popovers in collections #42512

Merged
merged 2 commits into from
May 10, 2024

Conversation

rafpaf
Copy link
Contributor

@rafpaf rafpaf commented May 10, 2024

With the collection table now inside a @container, popovers were being created above the bulk actions toast (z-axis-wise). This branch fixes this by changing which element serves as the container (we'll use CollectionTable, not CollectionRoot - both are equally good), and moving the bulk actions toast just outside this container.

Now the toast appears above popovers:

image.png

This branch also restores the margin-left adjustment that is needed to correct the position of the bulk actions toast when the sidebar is open. This adjustment was recently removed because the @container query fixes this problem, but now that the toast is not inside the @container, we need to restore that solution.

Copy link
Contributor Author

rafpaf commented May 10, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rafpaf and the rest of your teammates on Graphite Graphite

@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label May 10, 2024
@rafpaf rafpaf requested a review from iethree May 10, 2024 15:07
@rafpaf rafpaf marked this pull request as ready for review May 10, 2024 15:07
@graphite-app graphite-app bot requested a review from a team May 10, 2024 15:07
@graphite-app graphite-app bot added the no-backport Do not backport this PR to any branch label May 10, 2024
@rafpaf rafpaf marked this pull request as draft May 10, 2024 15:07
Copy link

graphite-app bot commented May 10, 2024

Graphite Automations

"Don't backport" took an action on this PR • (05/10/24)

1 reviewer was added and 1 label was added to this PR based on Raphael Krut-Landau's automation.

@rafpaf rafpaf changed the title q Put bulk actions above popovers in collections May 10, 2024
@rafpaf rafpaf marked this pull request as ready for review May 10, 2024 15:14
@rafpaf rafpaf removed the request for review from a team May 10, 2024 15:18
Copy link

replay-io bot commented May 10, 2024

Status In Progress ↗︎ 51 / 52
Commit 97bb00a
Results
⚠️ 4 Flaky
2479 Passed

Copy link
Contributor

@iethree iethree left a comment

Choose a reason for hiding this comment

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

nice! an elegant, simple solution.

@rafpaf rafpaf merged commit 1175ee6 into master May 10, 2024
166 of 173 checks passed
@rafpaf rafpaf deleted the put-bulk-actions-above-popovers-in-collections branch May 10, 2024 15:57
Copy link
Contributor Author

rafpaf commented May 10, 2024

Merge activity

Copy link

@rafpaf Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

@rafpaf rafpaf added this to the 0.50 milestone May 10, 2024
@rafpaf
Copy link
Contributor Author

rafpaf commented May 10, 2024

lol, I'm so sorry for tagging you, @container! I'm of course referring to https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries

uladzimirdev pushed a commit that referenced this pull request May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants