Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow MenuItems to work optionally when hidden #16853

Merged
merged 2 commits into from Feb 28, 2019

Conversation

@codebytere
Copy link
Member

commented Feb 8, 2019

Description of Change

Resolves #16747.

This PR enables MenuItems on macOS to work optionally when visible: false. using a new MenuItem option acceleratorWorksWhenHidden. The default of this new options is true, to match behavior on Windows and Linux, but since native macOS development allows for this to be turned off, this option allows that to be done.

Example:

  const menu = Menu.buildFromTemplate([
    {
      label: 'View',
      submenu: [
        { role: 'zoomin' },
        { 
          role: 'zoomout', 
          visible: false,
          acceleratorWorksWhenHidden: true
        },
      ],
    }
  ])

cc @MarshallOfSound @zcbenz

Checklist

Release Notes

Notes: Added an option to enable MenuItems on macOS to work optionally when visible: false.

@codebytere codebytere requested review from as code owners Feb 8, 2019

@@ -302,8 +307,7 @@ - (void)addItemToMenu:(NSMenu*)menu
}

// Called before the menu is to be displayed to update the state (enabled,
// radio, etc) of each item in the menu. Also will update the title if

This comment has been minimized.

Copy link
@codebytere

codebytere Feb 8, 2019

Author Member

comment removed as it's a remnant of dead code previously removed in #14939

@codebytere codebytere changed the title [wip] feat: allow MenuItems to work optionally when hidden feat: allow MenuItems to work optionally when hidden Feb 8, 2019

@codebytere codebytere force-pushed the menu-item-work-disabled branch from 3a5cdac to d69c03f Feb 8, 2019

@codebytere codebytere requested a review from brenca Feb 8, 2019

Show resolved Hide resolved atom/browser/api/atom_api_menu.cc Outdated
Show resolved Hide resolved docs/api/menu-item.md Outdated

@codebytere codebytere force-pushed the menu-item-work-disabled branch from 13eec15 to 598fb1f Feb 9, 2019

@brenca

brenca approved these changes Feb 26, 2019

Copy link
Member

left a comment

👍

@zcbenz

zcbenz approved these changes Feb 26, 2019

Show resolved Hide resolved docs/api/menu-item.md

@codebytere codebytere force-pushed the menu-item-work-disabled branch from d212262 to d2770a1 Feb 27, 2019

@codebytere codebytere force-pushed the menu-item-work-disabled branch from d2770a1 to cb3daa7 Feb 27, 2019

@MarshallOfSound

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Looks like you're gonna need one of those lovely Forward Declarations 😢

../../electron/atom/browser/ui/cocoa/atom_menu_controller.mm:296:11: error: instance method '-setAllowsKeyEquivalentWhenHidden:' not found (return type defaults to 'id') [-Werror,-Wobjc-method-access]
          setAllowsKeyEquivalentWhenHidden:(model->WorksWhenHiddenAt(index))];
@codebytere

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

@MarshallOfSound oops yeah i was planning on fixing this yesterday but then gclient died on me; fixing soon :)

@codebytere codebytere merged commit 544d8a4 into master Feb 28, 2019

15 of 16 checks passed

Artifact Comparison Changes Detected
Details
Backportable? - 5-0-x Clean Backport
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190228.6 succeeded
Details
electron-arm64-testing Build #20190228.6 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Feb 28, 2019

Release Notes Persisted

Added an option to enable MenuItems on macOS to work optionally when visible: false.

@trop

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

I have automatically backported this PR to "5-0-x", please check out #17175

@trop trop bot added in-flight/5-0-x and removed target/5-0-x labels Feb 28, 2019

@codebytere codebytere deleted the menu-item-work-disabled branch Feb 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.