-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fullscreen shortcut for non macOS #1754
Fullscreen shortcut for non macOS #1754
Conversation
Does it run on your windows/Linux ? The Maybe you should try to place it in the View submenu, and use the setFullScreen method instead 😄 |
@romainwn Good to know thanks, will get right on that :) |
Tested on windows and it works fine. |
Also tested on Windows and it works fine too. Maybe we should keep macOs as it is, and only make your changes for the non macOs platforms ? |
I think we should add another |
@yougotwill Could you fix it? |
@Rokt33r Sure thing will get on it this evening 👍 |
@romainwn @Rokt33r I changed the logic to be either of the fullscreen methods depending on the platform. Having said that @romainwn that error you mentioned about it focusing on the |
… platform" This reverts commit b46b958.
Ok so that change doesn't work on windows. So I have reverted the commit and will leave it until we find out if the issue comes from the master branch. |
I'll try to work on this feature as well today ! |
@romainwn ping. |
oh yup sorry for the delay. I'll try to make a PR by this week end, quite busy for the next two days ! |
I'll wait for it gladly. |
Sooo, I'm working on it right now, and found something interesting :
So on macOs, I noticed this, if I do Could we remove the F shortcut and only let the S one for the focus ? Meanwhile, I'm working on the windows/macOs full screen issue |
I'd like to suggest we change the Also @romainwn are you referring to this issue #1803 specifically a shortcut for the editor fullscreen option? |
Oh didn't notice this issue. For the FullScreen shortcut I was thinking to use |
This branch does exactly that :) All we need to do is change the |
So I have tested by changing the |
@yougotwill I don't think so. Just update this branch. And I agree changing shortcut key for |
@Rokt33r Changes done, and I have tested on windows and everything works correctly 👍 |
LGTM! 👍 |
Feature added for #1711
Fullscreen on macOS remains
Cmd+Ctrl+F
Fullscreen on Linux and Windows is
F11