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

Conditional menu build fix for window #1903

Merged
merged 2 commits into from
Jun 3, 2017
Merged

Conversation

ppot
Copy link
Contributor

@ppot ppot commented Jun 3, 2017

No description provided.

@ppot
Copy link
Contributor Author

ppot commented Jun 3, 2017

@Stanzilla You can test it.

@Stanzilla
Copy link
Contributor

Fixes it!

@@ -12,7 +12,6 @@ const darwinMenu = require('./menus/darwin');
module.exports = (createWindow, updatePlugins) => {
const commands = getKeymaps().commands;
const menu = [
(process.platform === 'darwin' ? darwinMenu(commands) : []),
Copy link
Contributor

@albinekb albinekb Jun 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...(process.platform === 'darwin' ? [darwinMenu(commands)] : []),

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you copy pasted it wrong 😉 #1876 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my comment was wrong too :D
...(process.platform === 'darwin' ? [darwinMenu(commands)] : []), this should work

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried (process.platform === 'darwin' ? [darwinMenu(commands)] : []), and it still causes a blank menu entry on Windows

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should keep '...' in the beginning to destructure (possible empty) array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! that makes it work indeed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will update

@chabou chabou merged commit ca84942 into vercel:master Jun 3, 2017
@albinekb
Copy link
Contributor

albinekb commented Jun 5, 2017

Nice @chabou @ppot @Stanzilla 💯 '

chabou pushed a commit to chabou/hyper that referenced this pull request Jun 9, 2017
@ppot ppot deleted the win_constricnt branch August 16, 2017 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants