Allow contextualActions to flow into the header toolbar
ClosedPublic

Authored by ahiemstra on Oct 14 2019, 3:08 PM.

Details

Summary

This replaces the custom layouting code in ToolBarPageHeader with
an ActionToolBar, which allows items from a page's contextualActions
to flow onto the page toolbar when there is space available.

One major change is that this adds support for alignment to
ActionToolBar, which effectively rewrites the layouting code for it.
Rather than use inline hidden actions, the new code uses a separate
hidden layout to determine action visibility.

Still to do (maybe in a followup patch?):

  • Add a priority property to Action that matches QAction's priority property.
  • Use the priority to determine if a certain action should be shown in the toolbar at all or always kept in overflow.
  • Use the priority to to replicate the icon only display behaviour used for main/left/right actions.
  • Find some way for actions to indicate whether to show text at all.

Demo of functionality:

Test Plan

Running kirigami2gallery everything should still work correctly, with
the addition of the mentioned action overflow.

However, behaviour with other applications will need to be checked,
which is the main reason this is marked as WIP.

Diff Detail

Repository
R169 Kirigami
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahiemstra created this revision.Oct 14 2019, 3:08 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptOct 14 2019, 3:08 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ahiemstra requested review of this revision.Oct 14 2019, 3:08 PM
ahiemstra updated this revision to Diff 67906.Oct 14 2019, 3:11 PM
  • Use the right width for showing the more button when all actions are visible
ahiemstra edited the summary of this revision. (Show Details)Oct 14 2019, 3:14 PM
ahiemstra edited the test plan for this revision. (Show Details)

Oh, this is so nice!

mart added a subscriber: mart.Wed, Oct 16, 2:53 PM

now has the order main action, left, right. before was left, main right.
it does change a bit the behavior, tough i see the point of making sure that the main action is always the last being hidden (i wonder how it would be making it hide last even if not the last)

mart added a comment.EditedWed, Oct 16, 3:00 PM

testing it i did find a major problem: if i resize the window quickly (kirigami gallery 2 columns mode) when it goes from icon+text to just icon, the window freezes for an instant, seems quite expensive, while before didn't do that

from the code nothing should have gotten particularly worse, so not sure how to debug

mart added a comment.Wed, Oct 16, 3:02 PM

Oh, this is so nice!

can you test/qa some existing apps? (at least discover and kamoso ?)

In D24634#548229, @mart wrote:

Oh, this is so nice!

can you test/qa some existing apps? (at least discover and kamoso ?)

Will do!

ngraham accepted this revision as: ngraham.Wed, Oct 16, 5:35 PM

This works really well in Discover, Kamoso, and kirigami-gallery. Still a big thumbs-up from me.

This revision is now accepted and ready to land.Wed, Oct 16, 5:35 PM

now has the order main action, left, right. before was left, main right.
it does change a bit the behavior, tough i see the point of making sure that the main action is always the last being hidden (i wonder how it would be making it hide last even if not the last)

While working on it, I realised that the left, main, right order actually doesn't make sense when placed in the toolbar, considering the main action is the most important action and normal order would be left-to-right with the most imporant on the leftmost place. It is easy enough to change back but I do consider this more "correct".

testing it i did find a major problem: if i resize the window quickly (kirigami gallery 2 columns mode) when it goes from icon+text to just icon, the window freezes for an instant, seems quite expensive, while before didn't do that
from the code nothing should have gotten particularly worse, so not sure how to debug

The main cause for this I believe is in the toolbar header file. It currently removes the overflow actions from the main action list when switching to the icon-only layout, which causes removal/recreation of hidden actions used for layout. I've been looking at implementing this behaviour in the actual actiontoolbar, which hopefully removes the need for all of that and speed up things. It's why this is still WIP. :)

mart accepted this revision.Mon, Oct 21, 10:55 AM

just remove the WIP from the review title then good to go

ahiemstra retitled this revision from [WIP] Allow contextualActions to flow into the header toolbar to Allow contextualActions to flow into the header toolbar.Mon, Oct 21, 1:22 PM
This revision was automatically updated to reflect the committed changes.