D22011: Add MenuSeparator

2019-06-23 Thread Alexander Stippich 
This revision was automatically updated to reflect the committed changes.
Closed by commit R858:6eb266c3d4fc: Add MenuSeparator (authored by astippich).

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22011?vs=60433=60528

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

AFFECTED FILES
  org.kde.desktop/MenuSeparator.qml

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-23 Thread Aleix Pol Gonzalez 
apol accepted this revision.
apol added a comment.


  LGTM
  
  To be honest, I would expect this to be the actual logic and 
Kirigami.Separator just use MenuSeparator, but maybe it's something we can look 
into in the future.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  menu_separator

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

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-23 Thread Alexander Stippich 
astippich updated this revision to Diff 60433.
astippich added a comment.


  - remove unneeded height

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22011?vs=60391=60433

BRANCH
  menu_separator

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

AFFECTED FILES
  org.kde.desktop/MenuSeparator.qml

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-23 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> apol wrote in MenuSeparator.qml:37
> It shouldn't need any height at all.

Oh duh, I was thinking it was a Rectangle, but it's a Kirigami.Separator

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  menu_separator

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

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-23 Thread Aleix Pol Gonzalez 
apol added inline comments.

INLINE COMMENTS

> ngraham wrote in MenuSeparator.qml:37
> @mart does this need to be multiplied by the device pixel ratio to account 
> for fractional scale factors? I forget.

It shouldn't need any height at all.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-22 Thread Filip Fila 
filipf added a comment.


  In the after picture I'm noticing that the separator is 1px outside the frame 
when the menus is over a dark bg.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: astippich, mart, apol, ngraham
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22011: Add MenuSeparator

2019-06-22 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> MenuSeparator.qml:37
> +width: controlRoot.width
> +height: 1
> +}

@mart does this need to be multiplied by the device pixel ratio to account for 
fractional scale factors? I forget.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

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

D22011: Add MenuSeparator

2019-06-22 Thread Alexander Stippich 
astippich added a comment.


  Related to D21944 
  Menu now looks like
  F6924938: menu_separator.png 
  
  before:
  F6924945: menu_before.png 

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

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

D22011: Add MenuSeparator

2019-06-22 Thread Alexander Stippich 
astippich created this revision.
astippich added reviewers: mart, apol, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
astippich requested review of this revision.

REVISION SUMMARY
  Make the MenuSeparator span the full width of the menu
  like in QWidget-based menus

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  menu_separator

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

AFFECTED FILES
  org.kde.desktop/MenuSeparator.qml

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