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

removes the dev mode access key binding safeguard #15044

Merged
merged 3 commits into from
Apr 26, 2019

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented Apr 18, 2019

Description

This removes the safeguard introduced in #14681 that would prevent devs to assign shortcuts containing alt + shift. All shortcuts that use the access key will be alt shift on linux and windows and ctrl alt on mac. Throwing therefore the error would require to check that the current env is apple but would not prevent someone on, say, linux to set it.

@ellatrix if you have any other idea it'd be great :)

@ellatrix
Copy link
Member

Check isAppleOS?

@oandregal
Copy link
Member

Experienced this problem myself, and can confirm removing the check fixes the issue. I'm open to a different approach if it still works on non-Apple OS :)

@oandregal
Copy link
Member

@ellatrix @draganescu any more thoughts to this? Is there any other approach? It's getting annoying not to be able to develop Gutenberg unless I have this patch applied :)

@draganescu draganescu force-pushed the update/remove-shortcut-dev-safeguard branch from 5c7659c to e4d7530 Compare April 23, 2019 19:43
@draganescu draganescu requested a review from ellatrix as a code owner April 23, 2019 19:43
@draganescu
Copy link
Contributor Author

I've redone this with adding a check for Apple platform, I would need someone to confirm it is working on other systems and not throwing an error anymore. @ellatrix if it looks good to you maybe you can approve this?

@oandregal
Copy link
Member

I can confirm this works in Ubuntu OS (Linux).

@draganescu
Copy link
Contributor Author

@ellatrix @nosolosw I think this can be unblocked now as it keeps the developer safeguard on and also works on other platforms and it handles that without useless exposed API.

/**
* Return true if platform is MacOS.
*
* @param {Object} _window window object by default; used for DI testing.
Copy link
Member

Choose a reason for hiding this comment

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

We're not using it here, so it can be removed?

Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

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

Let's make non-Apple users happy again!

@draganescu draganescu merged commit a34d52b into master Apr 26, 2019
@oandregal oandregal deleted the update/remove-shortcut-dev-safeguard branch April 26, 2019 14:53
@aduth aduth added the [Type] Bug An existing feature does not function as intended label Apr 29, 2019
@youknowriad youknowriad added this to the 5.6 (Gutenberg) milestone May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants