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.

Sign up for GitHub

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

Jump to bottom

Constant indicator width #255

Merged
merged 2 commits into from
May 18, 2023
Merged

Constant indicator width #255

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7b836c
Set width_request to 360
leolost2605 May 14, 2023
09eb791
Set width_request in Indicator.vala
leolost2605 May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Widgets/NotificationsList.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class Notifications.NotificationsList : Gtk.ListBox {

activate_on_single_click = true;
selection_mode = Gtk.SelectionMode.NONE;
width_request = 360;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does the value of 360 come from? I notice that in Indicator.vala , get_widget () returns a box with a width_request of 300.

Copy link
Member Author

@leolost2605 leolost2605 May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah right it might be better to change it there then.
360 is usually the width of the NotificationEntries that are the ListBoxRows representing Notifications but I can't say for sure that's always this way. Might this be problematic?

Copy link
Collaborator

@jeremypw jeremypw May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, changing it to 360 in Indicator.vala has the same effect without adding another line. I am not absolutely sure what limits the width of NotificationEntry - I'll do some testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like its the width_chars and max_width_chars properties of the title label.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After further investigation, it seems that the elementary notifications server produces a bubble with width 332 (hard coded). So if you want the notification list to match the bubbles, that value should be used in Indicator.vala. Maybe in future that value could be parameterized as a setting so both projects could be easily kept in sync (?)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we use character width here instead of width request so that if the text size changes you don't see fewer characters for large fonts.

It might be worth seeing if we can set a min-width with CSS in characters? Otherwise I wonder if we could pack an invisible fake notification maybe. Not super clean but would make sure it's the same width all the time

Copy link
Member Author

@leolost2605 leolost2605 May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After further investigation, it seems that the elementary notifications server produces a bubble with width 332 (hard coded). So if you want the notification list to match the bubbles, that value should be used in Indicator.vala. Maybe in future that value could be parameterized as a setting so both projects could be easily kept in sync (?)

The NotificationEntries are actually wider than that (they still look about the same my guess would be that's because of some CSS stuff which i don't really know much about), we probably would have to reduce the width_char then to keep a constant width which doesn't seem optimal.
I would still plead for increasing the width_request in Indicator.vala to 360 as this keeps the width constant most/all of the time (also with bigger text sizes), is very easy and not a very big change as the "default" size is hardcoded already anyway (at 300). The only problem I can see here is that the notifications indicator might take up more screen space which IMHO doesn't do to much harm at 60 units. Another problem is that it's still hardcoded so it might not work anymore when in the future the default sizes of NotificationEntries change.

set_placeholder (placeholder);
show_all ();

Expand Down