D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:872cfbcbd7cf: Add a mode to center actions and omit the 
title when using a ToolBar style (authored by ngraham).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23845?vs=65866=66230

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

AFFECTED FILES
  src/controls/PageRow.qml
  src/controls/private/globaltoolbar/PageRowGlobalToolBarStyleGroup.qml
  src/controls/private/globaltoolbar/ToolBarPageHeader.qml

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

ngraham added a comment.


  In D23845#532373 , @mart wrote:
  
  > One thing i forgot: how does this look when a title is present?
  
  
  It doesn't; `titleLoader` doesn't load the title when using centered 
alignment.

REPOSITORY
  R169 Kirigami

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Marco Martin]

mart accepted this revision.
mart added a comment.


  One thing i forgot: how does this look when a title is present?

REPOSITORY
  R169 Kirigami

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

ngraham updated this revision to Diff 65866.
ngraham added a comment.


  Fix typo

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23845?vs=65864=65866

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

AFFECTED FILES
  src/controls/PageRow.qml
  src/controls/private/globaltoolbar/PageRowGlobalToolBarStyleGroup.qml
  src/controls/private/globaltoolbar/ToolBarPageHeader.qml

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

ngraham added a comment.


  Thanks, hopefully this is better. Now there's a Qt.Alignment property. Not 
sure what you mean about the icon position enum though. Would appreciate 
clarification.

REPOSITORY
  R169 Kirigami

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

ngraham updated this revision to Diff 65864.
ngraham marked 2 inline comments as done.
ngraham added a comment.


  Make it better

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23845?vs=65790=65864

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

AFFECTED FILES
  src/controls/PageRow.qml
  src/controls/private/globaltoolbar/PageRowGlobalToolBarStyleGroup.qml
  src/controls/private/globaltoolbar/ToolBarPageHeader.qml

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Marco Martin]

mart added a comment.


  Like the concept, API may need a bit of improvement

INLINE COMMENTS

> PageRow.qml:146
>   * * showNavigationButtons: OR flags combination of 
> ApplicationHeaderStyle.ShowBackButton and 
> ApplicationHeaderStyle.ShowForwardButton
> + * * toolbarButtonsCenteredWithNoTitle: When using the ToolBar style, 
> center the actions and don't show a title
>   * * minimumHeight: (int) minimum height of the header, which will be 
> resized when scrolling, only in Mobile mode (default: preferredHeight, 
> sliding but no scaling)

This name for property is a bit... Meh
I would export a qt.alignment property *and* the icon position enum

> ToolBarPageHeader.qml:72
>  verticalCenter: parent.verticalCenter
> -right: ctxActionsButton.visible ? ctxActionsButton.left : 
> parent.right
>  }

Perhaps a state machine with AnchorChange may make it a bit better and reliable

REPOSITORY
  R169 Kirigami

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

To: ngraham, #kirigami, #vdg, mart, ndavis
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D23845: Add a mode to center actions and omit the title when using a ToolBar style

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Kirigami, VDG, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  This patch implements an optional and off-by-default feature to center the 
actions and
  omit the title when displaying the ToolBar style. This is useful for simple 
desktop apps
  that don't need to display a title in the toolbar, where centered toolbuttons 
look
  better than right-aligned toolbuttons.
  
  FEATURE: 404203
  CCBUG: 402948
  FIXED-IN: 5.63

TEST PLAN
  Apply this patch to Kirigami then run Kamoso after applying DXX
  
  Before: F7338499: Before.png 
  
  After: F7338498: After.png 

REPOSITORY
  R169 Kirigami

BRANCH
  optional-centered-actions-with-no-title (branched from master)

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

AFFECTED FILES
  src/controls/PageRow.qml
  src/controls/private/globaltoolbar/PageRowGlobalToolBarStyleGroup.qml
  src/controls/private/globaltoolbar/ToolBarPageHeader.qml

To: ngraham, #kirigami, #vdg, mart
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
davidedmundson, mart, hein