GNOME Bugzilla – Bug 755477
userList: ellipsize usernames
Last modified: 2015-10-02 07:52:52 UTC
Created attachment 311953 [details] screenshot of userlist containing too long name If I have a very long username, then we have some weird sizing behavior in the userList. It would be better to handle this case by ellipsizing the name. (this could be a good newcomer bug).
Created attachment 312474 [details] [review] userList: ellipsize usernames This patch added max_width_chars parameter to popover Gtk.Label Although there was ellipsize parameter for the Gtk.Label, it didn't work due to there was no limitation on Label size. max_width_chars adds necessary limitation to replace ending of long names to dots.
Created attachment 312476 [details] Demo of attachment 312474 [details] [review]
Review of attachment 312474 [details] [review]: >> const MAX_POPOVER_WIDTH = 17; Perhaps a more accurate name would be "MAX_USERNAME_WIDTH" ? >> Although there was ellipsize parameter for the Gtk.Label, it didn't work due to there was no limitation on Label size. max_width_chars adds necessary limitation to replace ending of long names to dots. You need to manually wrap the commit message at < 80 characters. :) The commit message is worded a bit funny to me. Maybe something like this? "The userListRow labels is set to ellipsize, but had no maximum width. We should set the max_width_chars for each label so longer labels ellipsize instead of making the user list grow beyond its default width."
Review of attachment 312474 [details] [review]: (In reply to jurikolo from comment #1) > Although there was ellipsize parameter for the Gtk.Label, it didn't work due > to there was no limitation on Label size. For context: It stopped working when the parent container no longer enforced a fixed width[0]. Some nits on the commit message: - subject should use header capitalization ("userList: Ellipsize usernames") - body should use present tense + active style: "Add max_width ..." instead of "This patch added max_width ...". - commit messages should contain line breaks at around 75 characters [0] https://git.gnome.org/browse/polari/commit?id=cf953db84bf29d ::: src/userList.js @@ +15,2 @@ const MAX_USERS_SHOWN = 8; +const MAX_POPOVER_WIDTH = 17; The name is misleading - "width" usually refers to pixel sizes, and it's used for the nick label rather than the popover itself. How about "MAX_NICK_WIDTH_CHARS" instead?
Created attachment 312487 [details] [review] userList: Ellipsize usernames Add max_width_chars parameter to popover Gtk.Label Username ellipsizing stopped working when the parent container no longer enforced a fixed width[0], so set GtkLabel:max-width-chars for the username label to make ellipsizing work again
Review of attachment 312487 [details] [review]: Sorry, still some minor nits in the commit message: > Add max_width_chars parameter to popover Gtk.Label The following sentence already says that slightly differently, so leave this out please. > the parent container no longer enforced a fixed width[0] "[0]" references the 0th footnote, but the footnote is missing - either add it, or remove the reference. However those are really just nits, so no need for another review - if you have git access, simply fix up the message before pushing, otherwise tell us and we'll push the patch for you. Thanks for the patch by the way :-) ::: src/userList.js @@ +15,2 @@ const MAX_USERS_SHOWN = 8; +const MAX_USERS_WIDTH_CHARS = 17; The consistency is a bit forced here, as "max-users" in the existing constant translates to "maximum number of users", while it's "maximum length of a single user (string)" in the new one. The context for each constant is obvious enough though, so OK to leave it like that.
(In reply to Florian Müllner from comment #6) > > the parent container no longer enforced a fixed width[0] > > "[0]" references the 0th footnote, but the footnote is missing - either add > it, or remove the reference. Fixed and pushed commit ebd0aa05843c9dd4b6a9cd422c8a4b227ba8ab53 to master now.