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.

Sign up for GitHub

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

Jump to bottom

adjusted menu to display zoom at view root level #863

Merged
merged 2 commits into from
Sep 20, 2018
Merged

adjusted menu to display zoom at view root level #863

merged 2 commits into from
Sep 20, 2018

Conversation

gie3d
Copy link
Contributor

@gie3d gie3d commented Sep 17, 2018

Based on #818

  • I changed Font Size Bigger, Smaller, to be Zoom In and Zoom Out and moved it to view root level.
  • Changed the short key for Zoom in from ⌘= to ⌘+

@mirka mirka self-requested a review September 17, 2018 18:30
mirka
mirka requested changes Sep 17, 2018
View reviewed changes
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

screen shot 2018-09-18 at 3 38 54

Thanks for your work on this! Looks great, and the plus/minus shortcuts are working. Just one minor issue about the Windows access keys, and we're good to go 🙂

desktop/menus/view-menu.js Outdated Show resolved Hide resolved
@mirka mirka mentioned this pull request Sep 17, 2018
Closed
11 tasks
@gie3d
Change Zoom In, Zoom Out and Toggle Full Screen access key on Windows…
9fc39da 
…. Zoom In to ‘i’ and Zoom Out to ‘o’. However, it will duplicate with Toggle Full Screen which previously was ‘o’, so I changed it to ‘f’
@gie3d
Copy link
Contributor Author

gie3d commented Sep 18, 2018 •
edited
Loading

Saw that the CI checks have fail. Looking through the errors and I think it's about deployment. Not sure that it is something that my code causes this trouble? and is there anything I can do to fix this?

mirka
mirka approved these changes Sep 18, 2018
View reviewed changes
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! As for the CI check failing, I'm not sure exactly... We're still working out the kinks in the CI system. I'll try to find out what's going wrong, but in any case I don't think it's a problem in your code, so no worries.

@mirka mirka merged commit 6c6d2c0 into Automattic:master Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirka mirka mirka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gie3d @mirka