Differential D16658

Respect the display property of buttons
ClosedPublic
Actions

Authored by astippich on Nov 4 2018, 9:52 AM.

Details

Reviewers
apol
mart
Group Reviewers
Plasma
Commits
R858:974efd584666: Respect the display property of buttons
Summary

Before, the display property of buttons introduced in Qt 5.10
was not taken into account. It allows to specify if the buttons
display icon or text only, or both. Text below icon is not yet
supported.

Diff Detail

Repository
R858 Qt Quick Controls 2: Desktop Style
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
astippich created this revision.Nov 4 2018, 9:52 AM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 4 2018, 9:52 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
astippich requested review of this revision.Nov 4 2018, 9:52 AM
apol added a comment.Nov 5 2018, 12:26 AM

I guess we can wait until we can bump to Qt 5.10 as a dependency? This looks a bit chaotic.
Also the Under/Beside setting isn't taken into account yet.

astippich added a comment.EditedNov 5 2018, 8:45 PM
In D16658#354233, @apol wrote:

I guess we can wait until we can bump to Qt 5.10 as a dependency? This looks a bit chaotic.

Reading https://community.kde.org/Frameworks/Policies, bumping to Qt 5.10 happens rather soonish, right?

Also the Under/Beside setting isn't taken into account yet.

Yep, forgot that to mention, but icon above the text is not something I currently care about.

astippich edited the summary of this revision. (Show Details)Nov 5 2018, 8:46 PM
astippich updated this revision to Diff 45424.Nov 13 2018, 5:01 PM
  • rebase
  • more verbose comment about compatibility
astippich added a comment.Nov 13 2018, 5:03 PM

Can I convince you to land this for Kf 5.53 with the promise to clean up once frameworks depends on Qt 5.10?

mart added a comment.Nov 23 2018, 2:02 PM
In D16658#359049, @astippich wrote:

Can I convince you to land this for Kf 5.53 with the promise to clean up once frameworks depends on Qt 5.10?

it needs at least a final release of Qt 5.12: frameworks need to depend from 2 Qt releases prior the current one.
however, i think it's possible to make it work without breaking old releases

mart requested changes to this revision.Nov 23 2018, 2:06 PM
mart added inline comments.
org.kde.desktop/Button.qml
44

controlRoot.hasOwnProperty("display")

63

controlRoot.display == undefined needs to become controlRoot.hasOwnProperty("display") this would be false with Qt 5.9 and less, protecting controlRoot.display != 1
also add the following comment:
//FIXME: when we can depend from Qt 5.10 do controlRoot.display !== T.AbstractButton.TextOnly

This revision now requires changes to proceed.Nov 23 2018, 2:06 PM
astippich updated this revision to Diff 46372.Nov 28 2018, 7:24 AM
  • use hasOwnProperty and add fixme comment
astippich edited the summary of this revision. (Show Details)Nov 28 2018, 7:24 AM
astippich updated this revision to Diff 48917.Jan 7 2019, 8:39 PM
  • remove compatibility for Qt 5.9 and cleanup
astippich added a comment.Jan 13 2019, 11:23 AM

friendly ping

astippich edited the summary of this revision. (Show Details)Jan 29 2019, 12:41 PM
astippich added a comment.Feb 11 2019, 8:15 PM

friendly ping

mart accepted this revision.Feb 12 2019, 3:50 PM
This revision is now accepted and ready to land.Feb 12 2019, 3:50 PM
Closed by commit R858:974efd584666: Respect the display property of buttons (authored by astippich). · Explain WhyFeb 12 2019, 8:57 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Morg.kde.desktop/Button.qml (4 lines)

Diff 51546

View Options

org.kde.desktop/Button.qml

Loading...