D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:9665afb2a594: Make it possible to show the title despite 
having ctx actions (authored by apol).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28727&id=28774

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/private/PrivateActionToolButton.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Looks good now! With your latest update, the right padding issue now actually 
resolved. The follow-up patch only needs to fix the left padding for the menu 
items that don't have checkboxes.

REPOSITORY
  R169 Kirigami

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28727.
apol added a comment.


  Improve how we calculate the header title.
  Let's fix the right spacing issue in a separate review

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28726&id=28727

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/private/PrivateActionToolButton.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  The problems persist with `QT_SCALE_FACTOR=1.5`, and additionally the titles 
get inappropriately elided:
  
  F5741717: Persists.png 

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28726.
apol added a comment.


  Odd, they both look rather small to me, probably a high dpi issue to look 
into...

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28723&id=28726

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/private/PrivateActionToolButton.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  Hmm, now there's too much side padding!
  F5741687: Now there's too much.png 
  
  Can we make the padding identical on the top, bottom, and right side?
  
  Also, as you can see in the above screenshot, the menu items without 
checkboxes have zero left padding, so they touch the bounds of the menu.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28723.
apol added a comment.


  Address Nate's comments

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28393&id=28723

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/private/PrivateActionToolButton.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  Generally looks good. A few UI comments:
  
  - Let's use a hamburger icon as the default icon, rather than the three-dot 
icon. That's used elsewhere on the list to expose hidden actions; here, it's 
opening a standard drop-down menu, so let's use the more standard icon for it.
  - The button doesn't have enough right padding: F5737034: Not enough 
padding.png 
  - I'm not really satisfied with the presentation when the button has text, as 
seen above; there's no icon, and no downward-pointing arrow  to indicate that 
clicking the button will open a drop-down menu.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28393.
apol added a comment.


  Give the title some more space if there's no actions
  
  Someone please review the change

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28334&id=28393

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28334.
apol added a comment.


  fix logic

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28333&id=28334

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28333.
apol added a comment.


  Tweaking

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28332&id=28333

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28332.
apol added a comment.


  Rebase

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28330&id=28332

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28330.
apol added a comment.


  If there's only 1 contextualAction, show that one instead of a menu with just 
1 thing

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=28305&id=28330

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 28305.
apol added a comment.


  Improve usability of the patch overall

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27825&id=28305

BRANCH
  arcpatch-D10475_1

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  tests/actionsMenu.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27825.
apol added a comment.


  Center the title and leave actions to the left and ctx actions to the right

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27623&id=27825

BRANCH
  arcpatch-D10475

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol added a comment.


  In D10475#211781 , @abetts wrote:
  
  > I am in favor of placing the Titles left aligned, but only if we don't put 
more than 1 control to the left of the title. If we can't compromise on that, 
another option is to keep titles center-aligned.
  
  
  Problem with center-aligned is that it's hard to have it look centered having 
components growing on either side. I'll give it a go to having the actions on 
the left and the contextualActions on the right.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Andres Betts]

abetts added a comment.


  I am in favor of placing the Titles right aligned, but only if we don't put 
more than 1 control to the right of the title. If we can't compromise on that, 
another option is to keep titles center-aligned.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  The title is still the primary focus for this header UI, which means (for RTL 
languages at least), it needs to be left or center aligned. This means the 
actions have to be on the right, to make sure your eyes don't see them before 
the title.
  
  @abetts, what do you think?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27623.
apol added a comment.


  Rebase on master

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27167&id=27623

BRANCH
  arcpatch-D10475

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol added a comment.


  Now that we have a menu, we definitely need to look into this.
  
  Without patch: odd
  F5720319: Screenshot_20180220_154236.png 

  
  With patch: still odd
  F5720335: Screenshot_20180220_155606.png 


REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Marco Martin]

mart added a comment.


  In D10475#206002 , @ngraham wrote:
  
  > F5710868: Too much on the left 1.png  
It looks so unbalanced.
  
  
  would probably be better < | VLC media player | icons

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  Don't worry Henrik, you're all good. The underlying issue here is that we 
haven't actually done the necessary design work for this feature yet. The 
appropriate Kirigami HIG section is empty: 
https://community.kde.org/KDE_Visual_Design_Group/KirigamiHIG#Bars
  
  If we merge this patch, there's a significant chance that a great deal of it 
will have to be re-done later. Thankfully there shouldn't be much if any 
downstream fallout because I don't believe any clients are actually using this 
feature on the desktop, so the worst thing that could happen is that we might 
just need to re-do this work later. If that seems unpalatable, it might make 
more sense to his the brakes and decide on our design before we implement it!

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Henrik Fehlauer]

rkflx added a comment.


  Oops, I really didn't want to block progress here. I think the only thing 
which needs clarification is what "title" entails in relation to the rest of 
the app, perhaps with a couple of examplary use cases.
  
  Another approach would be to define the text element as a kind of "label", 
showing things like a filename or a mode/state. Then it would still need 
spacing, but not so much, it could get a smaller font.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  I agree with @rkflx. I think we need to step back here and do some more long, 
hard design work before we rush to implementation. The proposed approach will 
not work for most narrow headers as there will simply not be enough room for 
navigation buttons, a title, context actions, and the menu button.
  
  I'd like to get @abetts and #VDG  
involved in this process before we proceed, as I anticipate otherwise we'll 
just go around in circles with patches to implement, change, and undo the same 
things over and over again.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Henrik Fehlauer]

rkflx added a comment.


  @ngraham
  
  > In this patch, we are trying to solve the general case of how a navigation 
header with context actions should look.
  
  I see, but I'm afraid with only a single example (which should be solved 
differently anyway) it is going to be hard to reach a conclusion. I'm not sure 
about the purpose of the "title" element.
  
  > "worst case scenario" for a header with two navigation buttons, a title, 
and 2 or more context actions.
  
  I doubt a "title" with four more elements plus the required spacing will work 
at all as a title.
  
  > On another note, what would be your preference for title alignment: 
left-aligned, or centered?
  
  There are two qualities which make something a title: First and foremost, 
enough room to breathe, and less importantly a more pronounced font (bigger 
size, bolder). This means for me one of these situations:
  
  - lots of space title lots of space
  - title lots of space small icon possibly with text
  - small icon with no text at all lots of space title lots of space small icon 
with no text at all
  
  I would avoid anything involving the following in relation to the title:
  
  - small space
  - no space at all
  - more than two non-space items in addition to the title
  - | as a replacement for lots of space
  
  Otherwise the title element cannot perform its role properly and feels 
cramped. Unfortunately this means most of the screenshots above don't fit those 
requirements. On the plus side, it explains the "remaining issues" and 
weirdness you noted above.
  
  In your case I would think about a two-row approach, i.e. a row resembling a 
toolbar, and a separate row for the title. Once you only have a title and two 
non-text icons, it could collapse to a single row.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27167.
apol added a comment.


  Fix placement of the navigation arrows

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27129&id=27167

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/templates/ApplicationHeader.qml

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  @rkflx Not naive at all!
  
  In this patch, we are trying to solve the general case of how a navigation 
header with context actions should look. Once we implement this control in 
Discover, we will //definitely// be removing the title from the app page's use 
of it, since as you point out, it's redundant as the page content below it 
already shows the app name. We may also even remove the context actions and put 
them on the page too, in a non-scrolling header as you suggest. So this patch 
is really trying to work out the "worst case scenario" for a header with two 
navigation buttons, a title, and 2 or more context actions.
  
  On another note, what would be your preference for title alignment: 
left-aligned, or centered?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Henrik Fehlauer]

rkflx added a comment.


  Excuse the very naive question, but why is the title duplicated in the top 
bar in the first place? It looks very cluttered, unaligned, and jumps around 
depending on which buttons are in use.
  
  It seems the only situation a title on top would be useful is when the users 
scrolled down and thus hid the intro header, but that could be solved 
differently: Add a small non-scrolling header as soon as the real header got 
hidden when scrolling down, as found on many websites.
  
  I think it's pretty clear that there is too little space in the top bar for 
everything, so you have to dedicate a larger area to the controls anyway.
  
  Alternative idea: Feature current application name in window title only.
  
  (Note I'm only talking about the desktop version here.)

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: rkflx, mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a reviewer: abetts.
ngraham added a comment.


  Great, now we have the titles on the left, like @mart  wants. We're getting 
there! There are a couple remaining issues:
  
  - Needs a re-base as it doesn't apply cleanly onto current master
  - Now everything in the header is left-aligned. It looks okay in widescreen 
mode, but just doesn't work outside of widescreen mode: F5710867: Too much on 
the left 2.png  F5710868: Too much on the 
left 1.png  It looks so unbalanced. I 
really think we need to move the context actions over to the right. The left is 
overloaded with stuff.
  - When I merge master and resolve the merge conflicts, the context actions 
overlap the navigation buttons: F5710865: overlapping.png 


REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham, abetts
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27129.
apol added a comment.


  Make the title actually stay to the left

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27083&id=27129

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  For me this patch still has the titles aligned to the right, even after your 
change:
  
  F5710374: right.png 
  
  But as currently implemented, if we move the title over to the left as I'm 
asking for, then the above image would change too having **three** elements on 
the left, and **none** on the right. It would look visually unbalanced.
  
  I think the context items need to be moved over to the right side of the 
header to visually balance the navigation buttons and title that will go on the 
left.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham added a comment.


  No, but in Desktop mode, if the window is not widescreen, then the left side 
of the header currently gets navigation buttons: Back when you can go back, and 
after the first time you go back, Forward is shown, too.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol added a comment.


  In D10475#205609 , @ngraham wrote:
  
  > Also, what happens to the navigation buttons if you're not in widescreen 
view? You get navigation buttons, and then context actions, and then the title? 
That'll be an awful lot of content to have on the left side.
  
  
  When on a mobile device, we should not use this toolbar.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27083.
apol added a comment.


  Keep the title aligned to the left

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27074&id=27083

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  This patch makes the title right-aligned, which is non-standard with 
anything. Let's keep it left-aligned for now. Also, what happens to the 
navigation buttons if you're not in widescreen view? You get navigation 
buttons, and then context actions, and then the title? That'll be an awful lot 
of content to have on the left side.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Marco Martin]

mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R169 Kirigami

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami, mart
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27074.
apol added a comment.


  Have the 3-dots button rightmost

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27033&id=27074

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Marco Martin]

mart added a comment.


  I like it!
  tough i think the more actions button should always be on the right of 
everything else, including the title

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol added a comment.


  Here you can see what Discover looks like once it's adopted.
  
  F5708931: kirigami-normal.png 
  
  F5708930: kirigami-tonofactions.png 

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D10475

To: apol, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol updated this revision to Diff 27033.
apol added a comment.


  Remove unrelated changes

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27032&id=27033

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein

D10475: Make it possible to show the title despite having ctx actions

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: Kirigami.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  At the moment, the ToolBarApplicationHeader doesn't show any title if there's
  actions on the toolbar, which makes for slightly weird empty space and looks
  inconsistent with other pages.
  We may want to make it configurable

REPOSITORY
  R169 Kirigami

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10475

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/private/PrivateActionToolButton.qml
  src/controls/templates/OverlaySheet.qml

To: apol, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein