Skip to content

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Oct 6, 2025

Initially, we had an issue that the feedback button was displayed outside the container when we showing a placeholder:

Screenshot 2025-10-06 at 19 44 28

There was an attempted fix by @shayna-ch here:

which seemed to work, however, it relies on overflow:hidden, which doesn’t work well in chonk because it cuts-off our buttons:

Screenshot 2025-10-06 at 19 44 01

What the fix tried to do was to limit the width of the container and force the content to not exceed its bounds with overfow:hidden.

However, the root cause seems to be that we are exceeding the bounds in the first place. Why is that?

It’s because the Placeholder component has a default width of width:100%, which is not what we want when there’s content next to it.
Luckily, we are in a flex layout, so what we really want to do is have the <Placeholder> use all the remaining width, which we can do with flexGrow:1.

I have therefore removed the GroupingInfoContainer that was introduced in #98928 and reverted back to a Fragment and fixed the issue by unsetting the width of the Placeholder and applying flexGrow:1 to it instead.

Screenshot 2025-10-06 at 19 43 40

Note: It’s weird that Placeholder has a default width of 100% - I don’t think we should be using width:100% in flex layouts, but I didn’t dare changing that inside Placeholder because it has so many usages.

Copy link

linear bot commented Oct 6, 2025

@TkDodo TkDodo requested a review from shayna-ch October 6, 2025 17:54
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 6, 2025
@TkDodo TkDodo requested a review from a team October 6, 2025 17:54
@TkDodo TkDodo marked this pull request as ready for review October 6, 2025 17:54
Copy link
Member

@shayna-ch shayna-ch left a comment

Choose a reason for hiding this comment

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

Thank you for catching this!

@TkDodo TkDodo merged commit 57c8c86 into master Oct 6, 2025
47 checks passed
@TkDodo TkDodo deleted the tkdodo/fix/de-278-button-cutoff-issue-due-to-overflow branch October 6, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants