D27996: Fix undefined check in global drawer menu mode

2020-03-14 Thread Nicolas Fella
This revision was automatically updated to reflect the committed changes.
Closed by commit R169:4d84d3f1d93f: Fix undefined check in global drawer menu 
mode (authored by nicolasfella).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27996?vs=77465=77630

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

AFFECTED FILES
  src/controls/private/globaltoolbar/PageRowGlobalToolBarUI.qml

To: nicolasfella, #kirigami, mart, ngraham
Cc: apol, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ahiemstra, davidedmundson, mart


D27996: Fix undefined check in global drawer menu mode

2020-03-11 Thread Nicolas Fella
nicolasfella marked an inline comment as done.
nicolasfella added inline comments.

INLINE COMMENTS

> apol wrote in PageRowGlobalToolBarUI.qml:49
> Wouldn't it be easier to do:
> 
>   visible: !Kirigami.Settings.isMobile && applicationWindow().globalDrawer && 
> applicationWindow().globalDrawer.isMenu
> 
> ?
> 
> Otherwise we better check for the property with `"isMenu" in 
> applicationWindow().globalDrawer`

The former gives `Unable to assign [undefined] to bool`

REPOSITORY
  R169 Kirigami

BRANCH
  fixkamoso

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

To: nicolasfella, #kirigami, mart, ngraham
Cc: apol, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ahiemstra, davidedmundson, mart


D27996: Fix undefined check in global drawer menu mode

2020-03-11 Thread Nicolas Fella
nicolasfella updated this revision to Diff 77465.
nicolasfella added a comment.


  - use 'in'

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27996?vs=77458=77465

BRANCH
  fixkamoso

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

AFFECTED FILES
  src/controls/private/globaltoolbar/PageRowGlobalToolBarUI.qml

To: nicolasfella, #kirigami, mart, ngraham
Cc: apol, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ahiemstra, davidedmundson, mart


D27996: Fix undefined check in global drawer menu mode

2020-03-11 Thread Aleix Pol Gonzalez
apol added inline comments.

INLINE COMMENTS

> PageRowGlobalToolBarUI.qml:49
>  id: menuButton
> -visible: !Kirigami.Settings.isMobile && 
> applicationWindow().globalDrawer && applicationWindow().globalDrawer.isMenu 
> !== "undefined" && applicationWindow().globalDrawer.isMenu
> +visible: !Kirigami.Settings.isMobile && 
> applicationWindow().globalDrawer && applicationWindow().globalDrawer.isMenu 
> !== undefined && applicationWindow().globalDrawer.isMenu
>  icon.name: "application-menu"

Wouldn't it be easier to do:

  visible: !Kirigami.Settings.isMobile && applicationWindow().globalDrawer && 
applicationWindow().globalDrawer.isMenu

?

Otherwise we better check for the property with `"isMenu" in 
applicationWindow().globalDrawer`

REPOSITORY
  R169 Kirigami

BRANCH
  fixkamoso

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

To: nicolasfella, #kirigami, mart, ngraham
Cc: apol, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ahiemstra, davidedmundson, mart


D27996: Fix undefined check in global drawer menu mode

2020-03-11 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Hah I was just investigating this the other day but couldn't find it. This 
makes sense.

REPOSITORY
  R169 Kirigami

BRANCH
  fixkamoso

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

To: nicolasfella, #kirigami, mart, ngraham
Cc: ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, 
ahiemstra, davidedmundson, mart


D27996: Fix undefined check in global drawer menu mode

2020-03-11 Thread Nicolas Fella
nicolasfella created this revision.
nicolasfella added reviewers: Kirigami, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
nicolasfella requested review of this revision.

REVISION SUMMARY
  Kamoso uses Overlaydrawer instead of GlobalDrawer, which has no isMenu 
property
  
  BUG: 417956

TEST PLAN
  Kamoso doesn't have an empty second hamburger button any more

REPOSITORY
  R169 Kirigami

BRANCH
  fixkamoso

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

AFFECTED FILES
  src/controls/private/globaltoolbar/PageRowGlobalToolBarUI.qml

To: nicolasfella, #kirigami, mart
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart