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

macOS: make the menu more like a macOS menu #1348

Merged
merged 4 commits into from
Apr 1, 2019

Conversation

tessus
Copy link
Collaborator

@tessus tessus commented Mar 16, 2019

@tessus tessus added the desktop All desktop platforms label Mar 16, 2019
@laurent22
Copy link
Owner

To avoid duplicate code even more, why not put each item that might move in a variable, and then insert it at the right location?

For example:

const syncStatusItem = {
	label: _('Synchronisation status'),
	click: () => {
		this.dispatch({
			type: 'NAV_GO',
			routeName: 'Status',
		});
	}
}

and then insert syncStatusItem at the relevant place. What do you think?

@tessus
Copy link
Collaborator Author

tessus commented Mar 28, 2019

What do you think?

Great idea. My bad that I missed this in the first place. I will change it accordingly. Oh, one more thing, there was a request to move some of the items to a File menu on macOS. Please see the following comment in the forum. Let me know, if you want to do this or not.
(Since I'm already playing around with this, I might as well do that too.)

@laurent22
Copy link
Owner

Yes I saw this comment, if you can do this as part of the pull request too that would be perfect.

@tessus
Copy link
Collaborator Author

tessus commented Mar 29, 2019

I will look at it tonight.

@tessus
Copy link
Collaborator Author

tessus commented Mar 30, 2019

image

image

@laurent22
Copy link
Owner

Perfect, thanks @tessus!

@laurent22 laurent22 merged commit db04906 into laurent22:master Apr 1, 2019
@tessus tessus deleted the macos-menu branch April 1, 2019 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants