D3805: Per-activity favorites (Final?)

[Eike Hein]

hein added a comment.


  ‎IRC log of review session:
  
  [05:37] ‎<‎Sho_‎>‎ ivan|home: currently compiling fav patch
  ‎[05:38] ‎<‎Sho_‎>‎ ivan|home: i may have one request
  ‎[05:38] ‎<‎ivan|home‎>‎ shoot
  ‎[05:39] ‎<‎Sho_‎>‎ ivan|home: we might have to renege on renaming favmodel to simplefavmodel - we technically don't really promise any api stability for the kicker backend, but we do have quite a few widgets on the KDE Store now that will break when the class changes name
  ‎[05:40] ‎<‎Sho_‎>‎ hmm actually it looks like Simple Menu wouldn't break
  ‎[05:40] ‎<‎Sho_‎>‎ maybe I should check the others first
  ‎[05:40] ‎<‎ivan|home‎>‎ Sho_: that is fine, the reason for the rename was for me to make sure I don't forget to replace the old model somewhere it should be replaced - to make the compiler complain :)
  ‎[05:41] ‎<‎Sho_‎>‎ oh, qmlRegisterType<SimpleFavoritesModel>(uri, 0, 1, "FavoritesModel");
  ‎[05:41] ‎<‎Sho_‎>‎ you didn't actually change the export name
  ‎[05:41] ‎<‎Sho_‎>‎ so it won't even break Kickoff
  ‎[05:41] ‎<‎Sho_‎>‎ I guess we can just swing it that way, too
  ‎[05:41] ‎<‎ivan|home‎>‎ yes, for qml - everything is the same
  ‎[05:42] ‎<‎Sho_‎>‎ alright
  ‎[05:42] ‎<‎ivan|home‎>‎ the KAStatsFavoritesModel might use a better name though
  ‎[05:42] ‎<‎Sho_‎>‎ restarting plasma
  ‎[05:42] ‎<‎Sho_‎>‎ "ActivityFavoritesModel" works for me
  ‎[05:43] ‎<‎Sho_‎>‎ Application Dashboard didn't migrate favorites for me
  ‎[05:43] ‎<‎Sho_‎>‎ but it's possible plasma hadn't flushed to config yet
  ‎[05:43] ‎<‎ivan|home‎>‎ That name might be a bit problematic since those are not favourite activities
  ‎[05:44] ‎<‎Sho_‎>‎ i think i just saw a bug
  ‎[05:44] ‎<‎ivan|home‎>‎ cool :)
  ‎[05:44] ‎<‎Sho_‎>‎ i had five favorites, and i added one to the current activity, and it appeared at index 4
  ‎[05:45] ‎<‎Sho_‎>‎ so not at the end, one item before the previously last one
  ‎[05:45] ‎<‎ivan|home‎>‎ one potential problem with the transitioning mech is that it will get only the items from the first menu that initializes
  ‎[05:46] ‎<‎Sho_‎>‎ interestingly after some activity switching it changed order
  ‎[05:46] ‎<‎ivan|home‎>‎ is your .config/kactivitymanagerd-statsrc empty?
  ‎[05:46] ‎<‎Sho_‎>‎ no
  ‎[05:47] ‎<‎Sho_‎>‎ plasma just crashed during favorites dnd
  ‎[05:47] ‎<‎ivan|home‎>‎ The ordering is a bit tricky - handling between global favs and per-activity favs - I'll check what are the things that can go wrong
  ‎[05:47] ‎<‎ivan|home‎>‎ bt?
  ‎[05:47] ‎<‎Sho_‎>‎ https://paste.kde.org/pnqer4fz7
  ‎[05:48] ‎<‎Sho_‎>‎ i was dragging and nto running though, hmm
  ‎[05:48] ‎<‎Sho_‎>‎ crashed again
  ‎[05:48] ‎<‎Sho_‎>‎ press, move, crash on release
  ‎[05:48] ‎<‎Sho_‎>‎ meanwhile in kicker no crash, but favorites also do not move
  ‎[05:49] ‎<‎ivan|home‎>‎ in AppEntry::run!?
  ‎[05:49] ‎<‎Sho_‎>‎ did you test favorites dnd with dashboard only or also with kicker?
  ‎[05:49] ‎<‎ivan|home‎>‎ KSycocaEntry pointer is null
  ‎[05:50] ‎<‎Sho_‎>‎ about the transitioning stuff
  ‎[05:50] ‎<‎Sho_‎>‎ is there some way we can change it to "combine all favorites from all menus and eliminate dupes"?
  ‎[05:50] ‎<‎Sho_‎>‎ i feel like users will be angry if they lose some faves
  ‎[05:50] ‎<‎ivan|home‎>‎ dnd with dashobard
  ‎[05:50] ‎<‎Sho_‎>‎ dnd in kicker doesn't seem to work at all
  ‎[05:51] ‎<‎ivan|home‎>‎ Sho_: I haven't used kicker in a while - will re-test
  ‎[05:51] ‎<‎Sho_‎>‎ also small nitpick (sorry if this is chaotic, best take notes) - should probably be "Show in Favorites", not capital I
  ‎[05:51] ‎<‎ivan|home‎>‎ The favs combining might be problematic, I'll see
  ‎[05:52] ‎<‎ivan|home‎>‎ We have Move To Desktop, Move To Activity in TM
  ‎[05:53] ‎<‎Sho_‎>‎ but also "Add to ..." in Kicker :D
  ‎[05:53] ‎<‎Sho_‎>‎ I'm also not sure what proper is :(
  ‎[05:53] ‎<‎Sho_‎>‎ but capitalizing articles and prepositions feels weird to me
  ‎[05:53] ‎<‎ivan|home‎>‎ We are as unified as ... to me as well - I did it because I saw it in other places :)
  ‎[05:54] ‎<‎ivan|home‎>‎ Sho_: why would dnd call trigger?
  ‎[05:55] ‎<‎Sho_‎>‎ it might be a bug in the ui code's mouse event handling
  ‎[05:55] ‎<‎Sho_‎>‎ it shouldn't call trigger 
  ‎[05:55] ‎<‎ivan|home‎>‎ for me, it does not
  ‎[05:55] ‎<‎Sho_‎>‎ but i also don't get any dnd happening btw
  ‎[05:56] ‎<‎Sho_‎>‎ oh wait
  ‎[05:56] ‎<‎Sho_‎>‎ i'm wondering if we're totally not on the same page here
  ‎[05:56] ‎<‎Sho_‎>‎ by "dnd" in both UIs I am referring to reordering favorites by drag
  ‎[05:56] ‎<‎Sho_‎>‎ so i move pointer to item, press, move while holding, icon doesn't move position, i release, crash in trigger
  ‎[05:57] ‎<‎ivan|home‎>‎ Kicker reordering works for me, and the dashboard ... :)
  ‎[05:57] ‎<‎ivan|home‎>‎ Do you have the latest master of everything?
  ‎[05:58] ‎<‎Sho_‎>‎ yep
  ‎[05:58] ‎<‎Sho_‎>‎ actually
  ‎[05:58] ‎<‎Sho_‎>‎ wait
  ‎[05:58] ‎<‎Sho_‎>‎ i might have forgotten to update kactivities
  ‎[05:58] ‎<‎Sho_‎>‎ also i didn't log out and back in
  ‎[05:58] ‎<‎Sho_‎>‎ ivan|home: can i restart the kactivities stuff easily without logging out?
  ‎[05:59] ‎<‎ivan|home‎>‎ kactivitymanagerd stop; kactivitymanagerd start
  ‎[05:59] ‎<‎ivan|home‎>‎ although that is a bit unsupported :)
  ‎[06:00] ‎<‎Sho_‎>‎ still no dnd :(
  ‎[06:00] ‎<‎Sho_‎>‎ what other dependencies might i need to update?
  ‎[06:00] ‎<‎ivan|home‎>‎ I'm trying to think of anything else... there should be no other deps
  ‎[06:01] ‎<‎ivan|home‎>‎ Hmh, something is wrong with Kicker for me. But the dashboard seems to work well.
  ‎[06:03] ‎<‎ivan|home‎>‎ Sho_: is Kicker showing the old model for you?
  ‎[06:04] ‎<‎Sho_‎>‎ no, i get the activity-poweredm enus
  ‎[06:05] ‎<‎Sho_‎>‎ that context menu btw has become an utter mess when you have recent docs and jump list actions in one
  ‎[06:05] ‎<‎Sho_‎>‎ after we merge this i might have a cleanup pass on the menu
  ‎[06:05] ‎<‎Sho_‎>‎ (not necessary to bog down this review with it)
  ‎[06:06] ‎* ‎ivan|home‎‎ hates when kdesrc-build replaces my branch with the master...
  ‎[06:08] ‎<‎ivan|home‎>‎ Sho_: ok, DnD and shared ordering needs to be fixed
  ‎[06:08] ‎<‎ivan|home‎>‎ DnD adding to favs works
  ‎[06:10] ‎<‎ivan|home‎>‎ Sho_: is there a flag that tells Kicker that the model is item-moveable instead of it starting to drag the mime type - this worked before I started reworking it for the dashboard
  ‎[06:11] ‎<‎ivan|home‎>‎ now it just creates a drag-object to drag it away from the menu into wherever
  ‎[06:14] ‎<‎Sho_‎>‎ ivan|home: in dashboard i think there is
  ‎[06:14] ‎<‎Sho_‎>‎ but that flag isn't set on the model, it's all handled in ui code, so the parameterized part is the qml "view" class
  ‎[06:14] ‎<‎Sho_‎>‎ the difference is basically between resulting in move() calls on the model or QDrag::exec
  ‎[06:15] ‎<‎ivan|home‎>‎ Sho_: Would you want me to put this into a branch in git?
  ‎[06:16] ‎<‎Sho_‎>‎ ivan|home: that's ok with me sure
  ‎[06:16] ‎<‎Sho_‎>‎ what i'll do now is dump this irc log into phab ;)
  ‎[06:16] ‎<‎ivan|home‎>‎ I guess it would be easier to test that way
  ‎[06:16] ‎<‎Sho_‎>‎ for posterity
  ‎[06:18] ‎<‎ivan|home‎>‎ ivan/new-favourites-per-activity

REVISION DETAIL
  https://phabricator.kde.org/D3805

To: ivan, mart, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170307/6f3d3cfc/attachment-0001.html>