Deskbar doesn't remember your expanded items when you switch modes

[jscipione]

In expanded mode with "Show application expander" checked in the preferences if you have expanded 1 or more items and you switch modes by dragging the window, Deskbar forgets your expanded items when you return to expanded mode.

Steps to reproduce. If you are not in expando then switch to expando mode (left or right but not top or bottom). Go to Deskbar preferences... in the feather menu Check the "Show application expander" checkbox if it is not already checked Drag the window to another mode, lets say top mode, then drag it back to expanded mode. Notice that your previously expanded items are no longer expanded.

[jscipione]

This patch creates a static list of expanded items. When ChangeState() is called the signatures of the expanded items are saved, Deskbar is rebuilt, then the items are re-expanded and the signature is deleted from the list

[jscipione]

Fix style issues in patch, take out non-related style changed, those are already fixed in #7052

[jscipione]

Updated patch after my other style change patches were committed

[jscipione]

Now that #7052 has been committed, the latest patch should be applied provided it meets the project's style requirements. This patch remembers the expanded state of your applications in Deskbar so that if you switch modes from vertical expando mode and back again your expanded applications will be re-expanded.

[jscipione]

ping

[axeld]

Sorry for the long delay! Is there any reason I don't see that the expanded list is static?

[jscipione]

Replying to axeld:

Is there any reason I don't see that the expanded list is static?

I assume that you mean in my patch. sExpandedItems is declared static in BarView.h

[jscipione]

If the expandNewTeams flag is set add new teams to the expanded list so that they will get expanded when you go to vertical expando mode. If you unexpanded the item already it will remember that and not re-expand it.

[jscipione]

Sorry to post a new patch after you had already starting looking at my old one but this one adds a new feature. If you are not in vertical expando mode, say you are in horizontal mode, and you have the expandNewTeams flag set in the preferences then when you switch into vertical expando mode any app that you opened in the other mode will automatically be expanded for you. Before this worked only if you were in vertical expando mode at the time that you opened the app.

[jscipione]

Add Tracker to the list of expanded items when you create the BarView if the expandNewTeams flag is set. This resolves #4830 in a simple way.

[axeld]

Replying to jscipione:

Replying to axeld:

Is there any reason I don't see that the expanded list is static?

I assume that you mean in my patch. sExpandedItems is declared static in BarView.h

What else could I possibly mean, anyway? Do you also have an answer for me? :-)

I don't really like the solution for #4830, it looks like a hack (directly accessing TBarView members), and seems out of place. I would rather like a callback like TBarView::TeamAdded() or something to that effect.

[jscipione]

Replying to axeld:

Replying to jscipione:

Replying to axeld:

Is there any reason I don't see that the expanded list is static?

I assume that you mean in my patch. sExpandedItems is declared static in BarView.h

What else could I possibly mean, anyway? Do you also have an answer for me? :-)

I don't know what else you could mean but I did answer your question I thought. sExpandedItems is declared static in BarView.h. Is that not good enough? Do I have to declare it static in BarView.cpp as well?

I don't really like the solution for #4830, it looks like a hack (directly accessing TBarView members), and seems out of place. I would rather like a callback like TBarView::TeamAdded() or something to that effect.

I guess it is kind of a hack to directly access the sExpendedItems from BarApp.cpp but I figured that a callback function would be overkill. Let me make another iteration of the patch that fixes this issue. I can make sExpandedItems private to BarView again if I use a callback method. Accessing the sExpandedItems member variable directly aside I am happy with the solution to #4830. It doesn't require any special functions to be written, just a simple check for the flag when BarView is created and add Tracker's signature to the list so that it gets expanded.

Thanks for looking at my patch and for your input axeld.

[jscipione]

Made the sExpandedItems list private again. Added a public method AddExpandedItem() to the BarView class that adds a signature to the expanded items list. Call that method anywhere I want to add an item to the sExpandedItems list.

[axeld]

You managed to misunderstood me: I see that sExpandedItems is static, I just don't see a reason why that is. So, why is it static?

[jscipione]

Replying to axeld:

You managed to misunderstood me: I see that sExpandedItems is static, I just don't see a reason why that is. So, why is it static?

I am sorry for the misunderstanding. The sExpandedItems list is static because I want there to be exactly one sExpandedItems list. I don't want to be able to accidentally create another sExpandedItems's list and start adding items to it. This is similar to other static lists such as sBarTeamInfoList and sSubscribers in BarApp.h. Of course you can only ever have one BarView per Deskbar (I hope) so perhaps I am being overly cautious. But since you can only ever have one BarApp per Deskbar I was copying the person who made those lists static. I assume that the author had something in mind when he or she mad those lists static and that I should copy him or her. I guess the only practical reason that I made sExpandedItems static was to avoid using an extern specifier when I declared sExpandedItems at the top of BarView.cpp.

[axeld]

I think this should be a resource that is tight to TBarView -- if you had two of them for whatever weird reason, why should they share the expander settings?

Also, if you actually had two TBarViews, the code as is wouldn't work any longer correctly, as there is no locking whatsoever.

For these reasons, I would prefer to make it a member instead.

[jscipione]

Since it has been 3 week since my last patch I wanted to give a status update less you think I am ignoring your comment. I have tried to make the sExpandedList non-static but I am running into a Deskbar crash on boot if you check the "Expand New Applications" checkbox in the preferences and you have open Tracker windows. (It is crashing while trying to add Tracker's sig to sExpandedItems on BarView construction.) I have no idea why changing the list to non-static would cause this to happen. Once I get that working I'll create a new patch. Thanks.

[jscipione]

Made sExpandedItems not static and renamed it fExpandedItems. When Deskbar starts, generally at startup, also after a crash or because it was explicitly quit and restarted items are expanded if you set ExpandNewTeams flag. This fixes #4830. Code cleanups abound. Fixed an off-by-one error in trunk when reading in the expanded items list. Axel, please look at this patch I believe that it is now ready to go. One last thing. Use C++ style static_cast rather than C style cast when casting to and from void* in the fExpandedItems BList. Ensure that UpdatePlacement() gets called at least once on startup.

[jscipione]

Looking back at patch 7 I can see that expanding the items could be made to work a bit faster by going through the fExpandedItems list backwards and deleting the signature from the list after expanding the corresponding team. That way on the next loop iteration that signature won't be checked again.

[jscipione]

Implemented the speed-up I commented on above. When expanding items go through the fExpandedItems list bottom up matching the fExpando list so that the items are in the same order. Also when a sig is found expand the item and then delete the signature from the list so that it is not considered in later loop iterations. This patch includes the same improvements of the last patch, it is just adds a small performance improvement.

[axeld]

Fixed in hrev43092 - please always note the revision of the fix if possible, John.