<table><tr><td style="">hein added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D3805" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>IRC log of review session:</p>
<p>[05:37] <Sho_> ivan|home: currently compiling fav patch<br />
[05:38] <Sho_> ivan|home: i may have one request<br />
[05:38] <ivan|home> shoot<br />
[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<br />
[05:40] <Sho_> hmm actually it looks like Simple Menu wouldn't break<br />
[05:40] <Sho_> maybe I should check the others first<br />
[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 :)<br />
[05:41] <Sho_> oh, qmlRegisterType<SimpleFavoritesModel>(uri, 0, 1, "FavoritesModel");<br />
[05:41] <Sho_> you didn't actually change the export name<br />
[05:41] <Sho_> so it won't even break Kickoff<br />
[05:41] <Sho_> I guess we can just swing it that way, too<br />
[05:41] <ivan|home> yes, for qml - everything is the same<br />
[05:42] <Sho_> alright<br />
[05:42] <ivan|home> the KAStatsFavoritesModel might use a better name though<br />
[05:42] <Sho_> restarting plasma<br />
[05:42] <Sho_> "ActivityFavoritesModel" works for me<br />
[05:43] <Sho_> Application Dashboard didn't migrate favorites for me<br />
[05:43] <Sho_> but it's possible plasma hadn't flushed to config yet<br />
[05:43] <ivan|home> That name might be a bit problematic since those are not favourite activities<br />
[05:44] <Sho_> i think i just saw a bug<br />
[05:44] <ivan|home> cool :)<br />
[05:44] <Sho_> i had five favorites, and i added one to the current activity, and it appeared at index 4<br />
[05:45] <Sho_> so not at the end, one item before the previously last one<br />
[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<br />
[05:46] <Sho_> interestingly after some activity switching it changed order<br />
[05:46] <ivan|home> is your .config/kactivitymanagerd-statsrc empty?<br />
[05:46] <Sho_> no<br />
[05:47] <Sho_> plasma just crashed during favorites dnd<br />
[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<br />
[05:47] <ivan|home> bt?<br />
[05:47] <Sho_> <a href="https://paste.kde.org/pnqer4fz7" class="remarkup-link" target="_blank" rel="noreferrer">https://paste.kde.org/pnqer4fz7</a><br />
[05:48] <Sho_> i was dragging and nto running though, hmm<br />
[05:48] <Sho_> crashed again<br />
[05:48] <Sho_> press, move, crash on release<br />
[05:48] <Sho_> meanwhile in kicker no crash, but favorites also do not move<br />
[05:49] <ivan|home> in AppEntry::run!?<br />
[05:49] <Sho_> did you test favorites dnd with dashboard only or also with kicker?<br />
[05:49] <ivan|home> KSycocaEntry pointer is null<br />
[05:50] <Sho_> about the transitioning stuff<br />
[05:50] <Sho_> is there some way we can change it to "combine all favorites from all menus and eliminate dupes"?<br />
[05:50] <Sho_> i feel like users will be angry if they lose some faves<br />
[05:50] <ivan|home> dnd with dashobard<br />
[05:50] <Sho_> dnd in kicker doesn't seem to work at all<br />
[05:51] <ivan|home> Sho_: I haven't used kicker in a while - will re-test<br />
[05:51] <Sho_> also small nitpick (sorry if this is chaotic, best take notes) - should probably be "Show in Favorites", not capital I<br />
[05:51] <ivan|home> The favs combining might be problematic, I'll see<br />
[05:52] <ivan|home> We have Move To Desktop, Move To Activity in TM<br />
[05:53] <Sho_> but also "Add to ..." in Kicker :D<br />
[05:53] <Sho_> I'm also not sure what proper is :(<br />
[05:53] <Sho_> but capitalizing articles and prepositions feels weird to me<br />
[05:53] <ivan|home> We are as unified as ... to me as well - I did it because I saw it in other places :)<br />
[05:54] <ivan|home> Sho_: why would dnd call trigger?<br />
[05:55] <Sho_> it might be a bug in the ui code's mouse event handling<br />
[05:55] <Sho_> it shouldn't call trigger <br />
[05:55] <ivan|home> for me, it does not<br />
[05:55] <Sho_> but i also don't get any dnd happening btw<br />
[05:56] <Sho_> oh wait<br />
[05:56] <Sho_> i'm wondering if we're totally not on the same page here<br />
[05:56] <Sho_> by "dnd" in both UIs I am referring to reordering favorites by drag<br />
[05:56] <Sho_> so i move pointer to item, press, move while holding, icon doesn't move position, i release, crash in trigger<br />
[05:57] <ivan|home> Kicker reordering works for me, and the dashboard ... :)<br />
[05:57] <ivan|home> Do you have the latest master of everything?<br />
[05:58] <Sho_> yep<br />
[05:58] <Sho_> actually<br />
[05:58] <Sho_> wait<br />
[05:58] <Sho_> i might have forgotten to update kactivities<br />
[05:58] <Sho_> also i didn't log out and back in<br />
[05:58] <Sho_> ivan|home: can i restart the kactivities stuff easily without logging out?<br />
[05:59] <ivan|home> kactivitymanagerd stop; kactivitymanagerd start<br />
[05:59] <ivan|home> although that is a bit unsupported :)<br />
[06:00] <Sho_> still no dnd :(<br />
[06:00] <Sho_> what other dependencies might i need to update?<br />
[06:00] <ivan|home> I'm trying to think of anything else... there should be no other deps<br />
[06:01] <ivan|home> Hmh, something is wrong with Kicker for me. But the dashboard seems to work well.<br />
[06:03] <ivan|home> Sho_: is Kicker showing the old model for you?<br />
[06:04] <Sho_> no, i get the activity-poweredm enus<br />
[06:05] <Sho_> that context menu btw has become an utter mess when you have recent docs and jump list actions in one<br />
[06:05] <Sho_> after we merge this i might have a cleanup pass on the menu<br />
[06:05] <Sho_> (not necessary to bog down this review with it)<br />
[06:06] * ivan|home hates when kdesrc-build replaces my branch with the master...<br />
[06:08] <ivan|home> Sho_: ok, DnD and shared ordering needs to be fixed<br />
[06:08] <ivan|home> DnD adding to favs works<br />
[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<br />
[06:11] <ivan|home> now it just creates a drag-object to drag it away from the menu into wherever<br />
[06:14] <Sho_> ivan|home: in dashboard i think there is<br />
[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<br />
[06:14] <Sho_> the difference is basically between resulting in move() calls on the model or QDrag::exec<br />
[06:15] <ivan|home> Sho_: Would you want me to put this into a branch in git?<br />
[06:16] <Sho_> ivan|home: that's ok with me sure<br />
[06:16] <Sho_> what i'll do now is dump this irc log into phab ;)<br />
[06:16] <ivan|home> I guess it would be easier to test that way<br />
[06:16] <Sho_> for posterity<br />
[06:18] <ivan|home> ivan/new-favourites-per-activity</p></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3805" rel="noreferrer">https://phabricator.kde.org/D3805</a></div></div><br /><div><strong>To: </strong>ivan, mart, hein<br /><strong>Cc: </strong>plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol<br /></div>