GNOME Bugzilla – Bug 787172
EmojiChooser: Improve selection of section buttons
Last modified: 2017-09-04 21:59:24 UTC
Created attachment 358989 [details] screenshot of EmojiChooser with 2 buttons active There are a couple of issues here: * It's possible to scroll and end up with the 1st two buttons being checked. This presumably only happens if the 1st, recent, has no emoji in it. Note that the recent section remains visible and takes a few pixels of space, even if there are no emoji in it. * The buttons are un/checked based on whether the scroll adjustment is currently within their heading. So you scroll a bit into a section, and now its button gets unselected - so no button is selected. Suggestions in turn: * Hide the recent section and button if there are no emoji in it. * Un/Check buttons based on the whole allocation of sections, not just headings.
Oh, also: * The Chooser opens with no button selected, but scrolled to the - currently empty - recent section.
Created attachment 358990 [details] screenshot of EmojiChooser with no buttons active
Created attachment 358995 [details] [review] EmojiChooser: Check the 1st button before 1st show We scroll to the top in show() but only ever selected a button when the adjustment changed, which doesn’t happen in init(). Check it manually. -- I'm still not sure it's useful to select the recent button when that section is initially empty and just taking up a few pixels of space, but while we scroll to that empty space, select its button accordingly.
Created attachment 358996 [details] [review] EmojiChooser: Ensure always have a selected button We were only selecting a section’s button if the adjustment y coord was within its heading, so scrolling slightly into it unchecked all buttons. This also fixes how we could end up with the first 2 selected, somehow. -- This could probably benefit from changing to the next section a little bit above it, but that will create confusion while we have an empty recent section. So, right now, immediately as you hit the top of the next section, its button checks
Created attachment 359000 [details] [review] EmojiChooser: Ensure always have a selected button We were only selecting a section’s button if the adjustment y coord was within its heading, so scrolling slightly into it unchecked all buttons. This also fixes how we could end up with the first 2 selected, somehow. -- avoids a declaration before statement, and other cosmetics
Created attachment 359095 [details] [review] EmojiChooser: Ensure always have a selected button We were only selecting a section’s button if the adjustment y coord was within its heading, so scrolling slightly into it unchecked all buttons. This also fixes how we could end up with the first 2 selected, somehow. -- now works! note to self: cosmetic cleanups break code. ugly code is best code
Created attachment 359099 [details] [review] EmojiChooser: Ensure always have a selected button We were only selecting a section’s button if the adjustment y coord was within its heading, so scrolling slightly into it unchecked all buttons. This also fixes how we could end up with the first 2 selected, somehow. -- Ditch the unusual looking backwards for loops, and try for a better name than "in_section".
Review of attachment 359099 [details] [review]: looks good to me now, thanks