Fixes the bug for me. I also reproduced it in 5.12 so you should add: FIXED-IN: 5.12.8

This is a good change, but it's more worth finding out why it doesn't work already. Looking at the code, when KickoffItem is triggered it runs the switchToInitial() function in FullRepresentation.qml, which already switches tabs and is also what sets the state back to Normal. So setting the tab in the state shouldn't be necessary. Can you debug why setting currentTab in switchToInitial() apparently doesn't work?

In D18848#407838, @hein wrote:

This is a good change, but it's more worth finding out why it doesn't work already. Looking at the code, when KickoffItem is triggered it runs the switchToInitial() function in FullRepresentation.qml, which already switches tabs and is also what sets the state back to Normal. So setting the tab in the state shouldn't be necessary. Can you debug why setting currentTab in switchToInitial() apparently doesn't work?

I think it might be because of the switch from the state "Search" to the state "Normal", because this doesn't happen if the arrow keys are used to navigate to the right of Favorites, only when Search is invoked. There's a "currentTab: searchPage" (line 738) setting in the state "Search" that probably gets stuck. I didn't want to deal with changing "Search" because in my experience, Kickoff's search view can be pretty temperamental. This seemed like the safest course of action.

BTW your FIXED-IN tag says 5.12.x, but this patch was branched off of master. So if and when this lands, if you don't do anything special, it's currently going to master only. To land it on 5.12.8, the easiest thing is to just cherry-pick the commit hash onto Plasma/5.12 and merge forward into 5.15 and master. I can help out with that.

In D18848#407897, @ngraham wrote:

BTW your FIXED-IN tag says 5.12.x, but this patch was branched off of master. So if and when this lands, if you don't do anything special, it's currently going to master only. To land it on 5.12.8, the easiest thing is to just cherry-pick the commit hash onto Plasma/5.12 and merge forward into 5.15 and master. I can help out with that.

Please do :D I find git confusing enough as it is :D

Can I use Kai's suggestion to git checkout Plasma/5.12 then commit then checkout Plasma/5.15 then merge Plasma/5.12, then checkout master then merge Plasma/5.15? Will this work if I cloned it off of anongit.kde.org (and not github.com)?

Regardless, let's wait for Eike's approval before landing this.

Here's how I would do it, if and when it's time to land this patch:

git checkout Plasma/5.12
git cherry-pick 2c3b1ee1995692aaa68e4d3c76e9bf37565741b2
git push
git checkout Plasma/5.14 (we're putting fixes on 5.14 too since we may ship another 5.14 bugfix release)
git merge Plasma/5.12
git push
git checkout Plasma/5.15
git merge Plasma/5.14
git push
git checkout master
git merge Plasma/5.15
git push
git branch -D fix-kickoff-initial-aftersearch

Wow thanks!
And after all is said and done, there's no need to run arc land?

Nah, arc land is just a fancy way of saying "take the commits on this branch and commit them onto the parent branch and then push the parent branch and then delete this branch"

You can always just do those things yourself if you'd like, which the above instructions show you how to do in a different way.

In D18848#407997, @ngraham wrote:

Nah, arc land is just a fancy way of saying "take the commits on this branch and commit them onto the parent branch and then push the parent branch and then delete this branch"

You can always just do those things yourself if you'd like, which the above instructions show you how to do in a different way.

I was just wondering about running arc land after the last push because it closes the diff? Or does git push do that too?

The diff is automatically closed once Phab detects a commit associated with it, so you don't have to do that manually (most of the time at least; sometimes it's a tad buggy).

In D18848#407999, @ngraham wrote:

The diff is automatically closed once Phab detects a commit associated with it, so you don't have to do that manually (most of the time at least; sometimes it's a tad buggy).

Excellent :D

In D18848#407843, @rooty wrote:

In D18848#407838, @hein wrote:

This is a good change, but it's more worth finding out why it doesn't work already. Looking at the code, when KickoffItem is triggered it runs the switchToInitial() function in FullRepresentation.qml, which already switches tabs and is also what sets the state back to Normal. So setting the tab in the state shouldn't be necessary. Can you debug why setting currentTab in switchToInitial() apparently doesn't work?

I think it might be because of the switch from the state "Search" to the state "Normal", because this doesn't happen if the arrow keys are used to navigate to the right of Favorites, only when Search is invoked. There's a "currentTab: searchPage" (line 738) setting in the state "Search" that probably gets stuck. I didn't want to deal with changing "Search" because in my experience, Kickoff's search view can be pretty temperamental. This seemed like the safest course of action.

Doesn't that mean the code in switchToInitial just needs reordering to set the state (to Normal) first and then do the tab change? (Or alternatively the tab change could be cleaned away from the function.)

Doesn't that mean the code in switchToInitial just needs reordering to set the state (to Normal) first and then do the tab change? (Or alternatively the tab change could be cleaned away from the function.)

You're right! That's a lot simpler than adding a property change, and it works.