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

Electron: Updates for Electron 1.2.8 #739

Merged
merged 4 commits into from
Jul 25, 2016
Merged

Electron: Updates for Electron 1.2.8 #739

merged 4 commits into from
Jul 25, 2016

Conversation

feross
Copy link
Member

@feross feross commented Jul 25, 2016

Followup PR to #738.

  • Electron: Use default labels and accelerators
  • macOS: add missing Edit menu roles
  • Use 'quit' role
  • Linux: Support showing badge count

Please review: @mathiasvr

feross added 4 commits July 25, 2016 15:21
Less code for us to maintain.

This also gives us free internationalization in a future Electron
version (they'll set the label dynamically based on the 'role')

One slight regression with this change, but it will be fixed in a
future Electron once this PR is merged:
electron/electron#6600
This was a macOS-only API before, but it's cross-platform now via
`app.setBadgeCount()`
@mathiasvr
Copy link
Contributor

mathiasvr commented Jul 25, 2016

LGTM 👍
But I think the undo/redo menu items are a little confusing. They are always active even though there is nothing to undo/redo, and they cannot be used to undo something like removing or adding a torrent. Can they actually be used for anything else than undoing/redoing text in the Open Torrent Address modal? If not, I would remove them.

Edit: Just realized that this applies to Cut, Copy, Delete and Select All as well! Maybe it is obvious that these items only applies to text, In that case we could keep them, but I would separate the Paste Torrent Address item.

@feross
Copy link
Member Author

feross commented Jul 25, 2016

We also have Cut/Copy/Select All which have the same problem.

It seems like most macOS apps tend to include these items even when they do nothing most of the time, so they can be used in text fields. I agree with you though. We should look at how we can disable these items when the user's not focused in a text field.

@feross feross merged commit 90a0201 into master Jul 25, 2016
@feross feross deleted the fixes-738 branch July 25, 2016 23:41
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants