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
