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

[NEW] Add missing Services menu in application menu on macOS #509

Merged
merged 1 commit into from
Aug 16, 2017
Merged

[NEW] Add missing Services menu in application menu on macOS #509

merged 1 commit into from
Aug 16, 2017

Conversation

flphvlck
Copy link
Contributor

@RocketChat/core

Closes #496

@gdelavald
Copy link
Contributor

Thanks for doing this PR. We'll evaluate and get back to you if there are any changes needed.

@flphvlck
Copy link
Contributor Author

Tested on macOS Sierra 10.12.6:
rocketchat_services

@gdelavald
Copy link
Contributor

@phidlipus is the Services menu expected to be populated automatically?

@flphvlck
Copy link
Contributor Author

@gdelavald Services menu will be populated only if you have configured some services. By default, if you haven't configured some services you will see only "Services Preferences".
The service menu should open, when you place your cursor on it.

@tomasgatial
Copy link

@gdelavald To my understanding, services menu is stateful, it should populate e.g. if you select a text within the application window. Then you can choose e.g. from text selection related services.

The electron documentation does say it is a system standard menu. Why keep it disabled?

@gdelavald
Copy link
Contributor

@phidlipus It's looking good! If I may add a suggestion, could you also add the services submenu to the context menu? I think that would improve the PR even further.
Right now the context menu is being modified in the SpellCheck.js file:

getMenu () {
return [
{
label: 'Undo',
role: 'undo'
},
{
label: 'Redo',
role: 'redo'
},
{
type: 'separator'
},
{
label: 'Cut',
role: 'cut',
accelerator: 'CommandOrControl+X',
},
{
label: 'Copy',
role: 'copy',
accelerator: 'CommandOrControl+C',
},
{
label: 'Paste',
role: 'paste',
accelerator: 'CommandOrControl+V',
},
{
label: 'Select All',
role: 'selectall',
accelerator: 'CommandOrControl+A',
}
];
}

I think it would be trivial to add the same submenu there.

@alexbrazier
Copy link
Contributor

@gdelavald I might be wrong, but I don't think its common to have the services menu in the context menu

@gdelavald
Copy link
Contributor

@alexbrazier I thought I saw some examples of it being useful. Haven't used osx long enough to be sure, I'll leave as is then, if someone feels like it could be useful we can discuss this in the future.

@alexbrazier alexbrazier merged commit ac38b94 into RocketChat:develop Aug 16, 2017
@flphvlck
Copy link
Contributor Author

Thanks for merge.

@gdelavald gdelavald changed the title Add missing Services menu in application menu on macOS [NEW] Add missing Services menu in application menu on macOS Aug 18, 2017
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.

Missing Services menu in application menu on macOS
4 participants