D17355: Align Plasma QML button content in center if it has an icon

2019-04-03 Thread Björn Feber 
GB_2 abandoned this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
cblack, arvidhansson, ian, hannahk, Pixel_Lime, jraleigh, squeakypancakes, 
alexde, IohannesPetros, GB_2, trickyricky26, ragreen, mglb, Pitel, michaelh, 
crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, 
jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Björn Feber 
GB_2 planned changes to this revision.
GB_2 added a comment.


  In D17355#376973 , @mart wrote:
  
  > in general, buttons with icons may be stacked vertically, which for me 
makes a general -1
  
  
  But it's not consistent with Qt Widgets and looks kind of weird.
  
  In D17355#376975 , @mart wrote:
  
  > In D17355#371722 , @GB_2 wrote:
  >
  > > Improved code.
  > >  How `qmlscene tests/components/button.qml` looks like:
  > >  F6457867: Plasma QML Button Content Align qmlscene (1).png 

  > >  For some reason "elide" doesn't work...
  >
  >
  > is elide working on the latest revision?
  
  
  No, I still didn't figure that out.
  
  In D17355#376976 , @mart wrote:
  
  > pleaase add the changes to plasmacomponents3 as well
  
  
  I will do that when this is done.
  
  This needs revision, but it's not really my priority.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  pleaase add the changes to plasmacomponents3 as well

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  In D17355#371722 , @GB_2 wrote:
  
  > Improved code.
  >  How `qmlscene tests/components/button.qml` looks like:
  >  F6457867: Plasma QML Button Content Align qmlscene (1).png 

  >  For some reason "elide" doesn't work...
  
  
  is elide working on the latest revision?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  in general, buttons with icons may be stacked vertically, which for me makes 
a general -1

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-08 Thread David Edmundson 
davidedmundson added a comment.


  > Any changes to PlasmaStyle need to happen in the QQC2 (PlasmaComponents3) 
version too before shipping

INLINE COMMENTS

> ButtonStyle.qml:41
> +label: Item {
> +implicitWidth: row.implicitWidth
> +RowLayout {

why this Item wrapper

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread Björn Feber 
GB_2 updated this revision to Diff 46920.
GB_2 added a comment.


  Improve menu button.
  F6458257: Plasma QML Button Content Align qmlscene (3).png 


REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17355?vs=46918=46920

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread Björn Feber 
GB_2 added a comment.


  Hmm... then I give up.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread David Edmundson 
davidedmundson added a comment.


  > I can't figure out how to make "elide" work without using Layout.fillWidth.
  
  You won't be able to. You need that.
  (or setting a max width, which effectively is the same thing)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread Björn Feber 
GB_2 updated this revision to Diff 46918.
GB_2 added a comment.


  I can't figure out how to make "elide" work without using `Layout.fillWidth`.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17355?vs=46907=46918

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread David Edmundson 
davidedmundson added a comment.


  On the button with menu, the triangle has moved. That seems an unwanted 
change.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread Björn Feber 
GB_2 marked an inline comment as done.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-05 Thread Björn Feber 
GB_2 updated this revision to Diff 46907.
GB_2 added a comment.


  Improved code.
  How `qmlscene tests/components/button.qml` looks like:
  F6457867: Plasma QML Button Content Align qmlscene (1).png 

  For some reason "elide" doesn't work...

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17355?vs=46862=46907

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread David Edmundson 
davidedmundson added a comment.


  Code wise:
  
  - Any changes to PlasmaStyle need to happen in the QQC2 (PlasmaComponents3) 
version too before shipping
  - Please be sure to run qmlscene tests/components/button.qml
  - Why do we have nested RowLayouts?
  
  
  
  > If we changed that so that buttons could be narrower when the icon + label 
is small, than we probably wouldn't need to horizontally center anything.
  
  That's do-able from application space.
  
  Button {
  
implicitWidth: minimumWidth
  
  }
  
  If you want that, there's no need to change this

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread Aleix Pol Gonzalez 
apol added inline comments.

INLINE COMMENTS

> ButtonStyle.qml:41
>  label: RowLayout {
> -spacing: units.smallSpacing
> +RowLayout {
> +Layout.alignment: Qt.AlignHCenter | Qt.AlignVCenter

This looks wrong, a RowLayout inside another one?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread Björn Feber 
GB_2 added a comment.


  It works properly, no matter how big the button is or if have an icon or not.
  In my opinion we should leave it how it is now.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread Björn Feber 
GB_2 updated this revision to Diff 46862.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17355?vs=46856=46862

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread Nathaniel Graham 
ngraham added a comment.


  Cool!
  
  Does this still work properly when the text is so long that the button 
expands to accommodate it?
  
  Now that I think about this a bit, it seems like maybe the problem here is 
that there's a minimum button width in the first place. If we changed that so 
that buttons could be narrower when the icon + label is small, than we probably 
wouldn't need to horizontally center anything.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

2018-12-04 Thread Björn Feber 
GB_2 created this revision.
GB_2 added reviewers: Plasma, VDG.
GB_2 added projects: Plasma, VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
GB_2 requested review of this revision.

REVISION SUMMARY
  Aligns the Plasma QML Button content in the center if it has an icon.
  It makes this look better: https://phabricator.kde.org/D17323
  
  Before: F6455121: Plasma QML Button Content Align (before).png 

  
  After: F6455122: Plasma QML Button Content Align (after).png 


TEST PLAN
  Run this test QML file with qmlscene: F6455115: main.qml 


REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, trickyricky26, 
ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, ngraham, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart