Show action.main more prominently on the ToolBarApplicationHeader
ClosedPublic

Authored by apol on May 31 2018, 1:56 PM.

Details

Summary

Show it as a full button instead of a tool button.
Use the icon color as the button color.

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.
apol created this revision.May 31 2018, 1:56 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptMay 31 2018, 1:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.May 31 2018, 1:56 PM
ngraham requested changes to this revision.May 31 2018, 2:03 PM
ngraham added a subscriber: ngraham.

Love it! Two issues that I can see:

  • In this example, the text really needs to be white; it doesn't show up well against the dark green background. If the color is programmatically determined, we may need some fancy logic to choose the button's text color to ensure that it's visible enough. For that matter, the icon doesn't show up very well either...
  • The button has no right padding, and touches the window edge ( A Discover issue?)
This revision now requires changes to proceed.May 31 2018, 2:03 PM

Love it! Two issues that I can see:

  • In this example, the text really needs to be white; it doesn't show up well against the dark green background. If the color is programmatically determined, we may need some fancy logic to choose the button's text color to ensure that it's visible enough. For that matter, the icon doesn't show up very well either...

Yeah... thought so too. If people are not against the idea it's something to spend time on. I'm afraid it could look weird though...

  • The button has no right padding, and touches the window edge ( A Discover issue?)

No, still in kirigami, it's just that with a non-flat button we don't take into account the padding within the button as a margin. We can add a margin.

apol updated this revision to Diff 35255.May 31 2018, 2:12 PM

Address some typos and added nate's margin

mart added inline comments.May 31 2018, 2:27 PM
src/controls/ToolBarApplicationHeader.qml
105

can just assign flat: false instead of adding a new property in actiontoolbutton?

mart requested changes to this revision.May 31 2018, 4:33 PM
mart added inline comments.
src/controls/private/PrivateActionToolButton.qml
62–66

note that this can't be used directly, as it depends from Qt 5.10
while if the button is colored by the method in D13232 this shouldn't happen
so this shouldn't be pushed until can be done without breaking deps

This revision now requires changes to proceed.May 31 2018, 4:33 PM
apol updated this revision to Diff 35296.Jun 1 2018, 12:00 AM

Remove property, only assign the palette if there's a palette property

mart accepted this revision.Jun 1 2018, 11:39 AM

Please remove console.log()

This revision was not accepted when it landed; it landed in state Needs Review.Jun 1 2018, 3:05 PM
This revision was automatically updated to reflect the committed changes.