do not force text display for ToolButton
ClosedPublic

Authored by astippich on Oct 17 2018, 8:33 PM.

Details

Summary

Do not force to always display the text besides the
icon for the ToolButton

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.Oct 17 2018, 8:33 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 17 2018, 8:33 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
astippich requested review of this revision.Oct 17 2018, 8:33 PM
astippich added a comment.EditedOct 17 2018, 8:36 PM

I noticed this while porting from qqc1 to qqc2 for Elisa, as the qqc2 ToolButton did not behave as expected. Neither qqc1 version nor the Fusion style display a text for a ToolButtons.
I don't know the reason why this was added and also have no clue about the implications this change may have in case some apps rely on that behavior.

astippich retitled this revision from do not force text for ToolButton to do not force text display for ToolButton.Oct 17 2018, 8:36 PM
astippich edited the summary of this revision. (Show Details)
ngraham added a subscriber: ngraham.Nov 1 2018, 6:49 PM

Wouldn't this change make it revert to the default value, which is Qt::ToolButtonIconOnly? How about Qt::ToolButtonFollowStyle instead? That way it'll pick up the user-overridable setting in the style KCM.

@mgallien stated in the meantime that we probably have been abusing the ToolButton in Elisa for our needs, and we will go a different route anyway.
Since the desktop style still behaves differently than QQC1 and the Fusion style, I think it's still of value to discuss this. I will just follow your call here, tell me what the right solution is :)

ngraham added a subscriber: hein.Nov 1 2018, 7:52 PM

To me, using Qt::ToolButtonFollowStyle would seem to make sense, but it might be best to ask @mart or @hein, who may know more of the history behind why Qt::ToolButtonTextBesideIcon was chosen here in the first place.

astippich updated this revision to Diff 45425.Nov 13 2018, 5:05 PM
  • follow style

friendly ping

davidedmundson accepted this revision.Jan 6 2019, 6:12 PM
davidedmundson added a subscriber: davidedmundson.

Seems sensible.

This revision is now accepted and ready to land.Jan 6 2019, 6:12 PM
This revision was automatically updated to reflect the committed changes.