Skip to content
This repository was archived by the owner on Dec 13, 2017. It is now read-only.

Added hardware keyboard shortcuts for iPad#820

Merged
frosty merged 5 commits intodevelopfrom
feature/keyboard-shortcuts
Jun 1, 2016
Merged

Added hardware keyboard shortcuts for iPad#820
frosty merged 5 commits intodevelopfrom
feature/keyboard-shortcuts

Conversation

@frosty
Copy link
Copy Markdown
Contributor

@frosty frosty commented May 16, 2016

Fixes #806. I've added keyboard shortcuts for iPads using hardware keyboards.

I've used shortcut keys they either: match system defaults (bold / underline), match other third party apps (bullet and ordered list shortcuts match iA Writer), or I've tried to pick something that makes sense where I couldn't find any precedent. Here's the initial selection that I've added:

keyboard-shortcuts

To test: Try to use each of the shortcuts listed above (you can hold down Command in the app to view the shortcuts). Test with text selected and without.

Also, please let me know if you think any of the shortcut keys should be changed to something else!

Needs Review: @SergioEstevao
Thanks!

@SergioEstevao
Copy link
Copy Markdown
Contributor

@frosty Do you mind adding these shortcuts to the legacy editor also?

Comment thread Classes/WPEditorViewController.m Outdated
[UIKeyCommand keyCommandWithInput:@"I"
modifierFlags:UIKeyModifierCommand|UIKeyModifierShift
action:@selector(didTouchMediaOptions)
discoverabilityTitle:NSLocalizedString(@"Insert Image", @"Discoverability title for insert image keyboard shortcut.")],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This shortcut should be renamed to be "Insert Media" and maybe change the shortcut to Cmd + M

@frosty
Copy link
Copy Markdown
Contributor Author

frosty commented May 17, 2016

@SergioEstevao I've managed to add support to the legacy editor. I had to add a UITextView subclass, and I've explained the reason for that in its documentation comments.

@SergioEstevao
Copy link
Copy Markdown
Contributor

@frosty I tested it a bit more on the device and found the following issues:

  • on the visual editor, the shortcuts don't work on the HTML mode
  • on the visual editor, when you use a shortcut, the corresponding icon on the toolbar should be activated. For bold, italic and underline the icons that are getting activated are the one on the new standard keyboard toolbar on the iPad, is this expected?

@frosty
Copy link
Copy Markdown
Contributor Author

frosty commented May 23, 2016

Thanks for the comments, @SergioEstevao – sorry for the delay in replying!

on the visual editor, when you use a shortcut, the corresponding icon on the toolbar should be activated

This is working for me – can you confirm that it's definitely not for you? I can record a GIF / video if necessary.

on the visual editor, the shortcuts don't work on the HTML mode

I've taken a look at this and it seems to be the same issue as with the legacy editor: HTML uses a UITextView, which suffers from the same iOS 9 bug. I'm just thinking about the best way to approach fixing this in both places (alongside working on other stuff), and I'll try and get something pushed up soon!

@frosty
Copy link
Copy Markdown
Contributor Author

frosty commented May 31, 2016

@SergioEstevao I've been looking into highlighting the toolbar buttons, and the problem only affects the Bold and Italic buttons. It's the same iOS 9 bug, where our custom methods for these shortcuts aren't called. The issue is that we seem to have no way of knowing then the user's pressed the keyboard shortcut. I've even hooked up the editor to Safari's inspector, and no javascript events get fired when you press Cmd+B/ Cmd+I.

It's up to you whether you want to proceed without these two buttons getting highlighted as soon as you press the shortcut? They do highlight once you start pressing other keys, and as you noted the B / I icons on the top of the keyboard get highlighted too. I think the benefit of having the shortcuts in the first place outweighs these few issues.

@SergioEstevao
Copy link
Copy Markdown
Contributor

Ok, let's proceed with this anyway I think it will be good for our users with keyboards, even if they are few of them. :shipit:

@frosty frosty merged commit 14d4b81 into develop Jun 1, 2016
@frosty
Copy link
Copy Markdown
Contributor Author

frosty commented Jun 1, 2016

Thanks @SergioEstevao!

@frosty frosty deleted the feature/keyboard-shortcuts branch June 1, 2016 08:26
@SergioEstevao SergioEstevao modified the milestones: 1.6.3, 1.7.0 Jun 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants