Skip to content

Conversation

@lenemter
Copy link
Member

@lenemter lenemter commented Oct 4, 2025

Fixes #237

image image image

@lenemter lenemter requested review from a team October 4, 2025 20:32
@ryonakano
Copy link
Member

It looks like the notification bubble has no bottom padding when one or more buttons that are ellipsized exist:

スクリーンショット 2025-10-13 10 23 19

child.measure (HORIZONTAL, -1, null, out child_nat_w, null, null);
child.measure (VERTICAL, -1, null, out child_nat_h, null, null);

if (row_used_width + child_nat_w + SPACING * (row.length () - 1) > width) {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry if I misunderstand, but why * (row.length () - 1) is necessary here……? I understand row_used_width is consist of (<natural width of child> + SPACING) * <number of child in this row> so I'm not sure why it's necessary.

Also I guess the calculation result of it could be -1 for the first time but is that expected……?

Comment on lines +100 to +101
// If the child doesn't fit in the current row and it's not the first in row -> wrap
if (row_used > 0 && prospective > width) {
Copy link
Member

Choose a reason for hiding this comment

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

What the comment says and what the actual code does are inverted here.

I think we should either

            // If the child isn't the first in row it doesn't fit in the current row -> wrap
            if (row_used > 0 && prospective > width) {

or

            // If the child doesn't fit in the current row and it's not the first in row -> wrap
            if (prospective > width && row_used > 0) {

@danirabbit
Copy link
Member

I think buttons should still be homogenous in width. So we either can fit all of them horizontally and lay them out that way or we should lay them all out vertically. But having a mix looks really chaotic.

See for example:

image

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.

Notification is too wide with long actions

4 participants