GNOME Bugzilla – Bug 723179
Support the extended folders schema
Last modified: 2014-01-29 19:05:20 UTC
This will allow us to reimplement folders like "Games" and "Wine" on the new system.
Created attachment 267422 [details] [review] appDisplay: Properly check for TerminalEmulator in the Categories key get_categories() returns an unparsed string, not a list of categories. We need to parse the list by splitting on ';' to deterine whether the actual 'TerminalEmulator' category is in the list, rather than something like 'X-GNOME-TerminalEmulator'. It's an edge case, but I need to split the list properly for the new folders, so I might as well fix this.
Created attachment 267423 [details] [review] appDisplay: Only reload frequently used data when mapping the frequent view If the user mostly uses the All Apps view and uses it as his default view, we shouldn't reload frequent data after a timeout. Simply do it when the view is mapped.
Created attachment 267424 [details] [review] appDisplay: Make AllView handle the display of the all apps view Rather than having the central component handle it. Just a code tidying-up.
Created attachment 267425 [details] [review] appDisplay: Make refiltering folders more efficient Rather than queueing a full redisplay, simply filter the apps inside folders by toggling the icon's visibility.
Created attachment 267426 [details] [review] appDisplay: Support the 'categories' key for app folders
Created attachment 267427 [details] [review] appDisplay; Support the 'excluded-apps' key for app folders
Review of attachment 267422 [details] [review]: Yes
Review of attachment 267424 [details] [review]: Yes
Review of attachment 267426 [details] [review]: ::: js/ui/appDisplay.js @@ +67,3 @@ + if (itemA == itemB) + return true; + } b.indexOf(itemA) >= 0 ?
Review of attachment 267427 [details] [review]: ::: js/ui/appDisplay.js @@ +1061,3 @@ + if (excludedApps.indexOf(appId) >= 0) + return; Kinda ugh looking at the linear search, I'd much rather use a hash map here, because this is called often if the folder is crowded. Nevertheless, I agree the excluded list should be empty or very limited anyway, so I'm ok with this.
Comment on attachment 267422 [details] [review] appDisplay: Properly check for TerminalEmulator in the Categories key Attachment 267422 [details] pushed as 887a21a - appDisplay: Properly check for TerminalEmulator in the Categories key
Created attachment 267430 [details] [review] appDisplay: Make AllView handle the display of the all apps view Rather than having the central component handle it. Just a code tidying-up.
Created attachment 267431 [details] [review] appDisplay: Make refiltering folders more efficient Rather than queueing a full redisplay, simply filter the apps inside folders by toggling the icon's visibility.
Created attachment 267432 [details] [review] appDisplay: Support the 'categories' key for app folders
Created attachment 267433 [details] [review] appDisplay: Support the 'excluded-apps' key for app folders
Created attachment 267434 [details] [review] appDisplay: Only reload frequently used data when mapping the frequent view If the user mostly uses the All Apps view and uses it as his default view, we shouldn't reload frequent data after a timeout. Simply do it when the view is mapped.
Review of attachment 267434 [details] [review]: Ok
Review of attachment 267430 [details] [review]: Yes, as before.
Review of attachment 267431 [details] [review]: ::: js/ui/appDisplay.js @@ +385,3 @@ + _resortGrid: function() { + this._grid.destroyAll(); + this.loadGrid(); As you mentioned, clutter_actor_new() is what takes most time in profile. But you can avoid it, if you push this resort() down to BaseIcon, and remove/sort/add the existing actors. @@ +1050,3 @@ + this.name = _getFolderName(this._folder); + this.icon.label.text = this.name; + this.emit('name-changed'); There is no point in two signals, if you always emit both of them. Instead, check if the name is different than before.
Review of attachment 267432 [details] [review]: Yes
Review of attachment 267433 [details] [review]: Ok
Created attachment 267446 [details] [review] appDisplay: Make refiltering folders more efficient Rather than queueing a full redisplay, simply filter the apps inside folders by toggling the icon's visibility.
Review of attachment 267446 [details] [review]: Awesome. Only moving the one element that changed is a nice touch.
Attachment 267430 [details] pushed as 3779ac2 - appDisplay: Make AllView handle the display of the all apps view Attachment 267432 [details] pushed as bb8fa61 - appDisplay: Support the 'categories' key for app folders Attachment 267433 [details] pushed as bb8397b - appDisplay: Support the 'excluded-apps' key for app folders Attachment 267434 [details] pushed as bdad4db - appDisplay: Only reload frequently used data when mapping the frequent view Attachment 267446 [details] pushed as 10147ee - appDisplay: Make refiltering folders more efficient