D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham
This revision was automatically updated to reflect the committed changes.
Closed by commit R263:93455f78d78d: Move Full Screen Mode item from 
Settings menu to View menu (authored by ngraham).

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24091?vs=66541=66554

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

AFFECTED FILES
  src/ui_standards.rc

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham
ngraham updated this revision to Diff 66541.
ngraham added a comment.


  Add safe extra separator

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24091?vs=66484=66541

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

AFFECTED FILES
  src/ui_standards.rc

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham
ngraham added a comment.


  Oh perfect, then this can land as-is. Thanks!

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread David Faure
dfaure added a comment.


  You can never end up with double separators, QMenu takes care of that.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Nathaniel Graham
ngraham added a comment.


  @cfeck can clarify, but I believe it specifies the location of locally-added 
actions that come from the app itself. It would be empty if the app has a View 
menu but doesn't add any of its own actions (which seems unlikely, but is 
theoretically possible).
  
  Alternatively we could remove the separator below redisplay, which would 
allow us to always have a separator above Full Screen Mode with no danger of 
ever having double separators.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Noah Davis
ndavis added a comment.


  In D24091#534786 , @ngraham wrote:
  
  > Do you think I should always have a separator above the Full Screen menu 
item? This should generally work fine, but it means that in case the 
`MergeLocal` content is empty, there will be two separators in a row. I don't 
know how likely this is to ever happen, but it could. Thoughts?
  
  
  I'm not sure what `MergeLocal` does or when it would be empty.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Nathaniel Graham
ngraham added a comment.


  Do you think I should always have a separator above the Full Screen menu 
item? This should generally work fine, but it means that in case the 
`MergeLocal` content is empty, there will be two separators in a row. I don't 
know how likely this is to ever happen, but it could. Thoughts?

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Noah Davis
ndavis accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

To: ngraham, #vdg, #frameworks, cfeck, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-19 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: VDG, Frameworks, cfeck.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  This is a pretty longstanding feature request and all the HIG and VDG people 
polled
  were in favor. Since those were the people requested to sign off on it, it 
looks like
  we can make the necessary code change now.

TEST PLAN
  F7387185: In the view menu.png 

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

AFFECTED FILES
  src/ui_standards.rc

To: ngraham, #vdg, #frameworks, cfeck
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns