D16284: do not force text display for ToolButton

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R858:d37015a5d1b0: do not force text display for ToolButton 
(authored by astippich).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16284?vs=45425&id=48915#toc

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16284?vs=45425&id=48915

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

AFFECTED FILES
  plugin/kquickstyleitem.cpp

To: astippich, mart, apol, davidedmundson
Cc: davidedmundson, hein, mgallien, ngraham, plasma-devel, kvanton, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16284: do not force text display for ToolButton

[David Edmundson]

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


  Seems sensible.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  toolbutton

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

To: astippich, mart, apol, davidedmundson
Cc: davidedmundson, hein, mgallien, ngraham, plasma-devel, kvanton, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D16284: do not force text display for ToolButton

[Alexander Stippich]

astippich added a comment.


  friendly ping

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol
Cc: hein, mgallien, ngraham, plasma-devel, kvanton, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D16284: do not force text display for ToolButton

[Alexander Stippich]

astippich updated this revision to Diff 45425.
astippich added a comment.


  - follow style

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16284?vs=43823&id=45425

BRANCH
  toolbutton

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

AFFECTED FILES
  plugin/kquickstyleitem.cpp

To: astippich, mart, apol
Cc: hein, mgallien, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16284: do not force text display for ToolButton

[Nathaniel Graham]

ngraham added a subscriber: hein.
ngraham added a comment.


  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.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol
Cc: hein, mgallien, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16284: do not force text display for ToolButton

[Alexander Stippich]

astippich added a subscriber: mgallien.
astippich added a comment.


  @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 :)

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol
Cc: mgallien, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16284: do not force text display for ToolButton

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16284: do not force text display for ToolButton

[Alexander Stippich]

astippich added a comment.


  opinions?

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart