Skip to content

Add keyboard shortcuts. #566

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
Merged
merged 3 commits into from
Jun 26, 2017
Merged

Add keyboard shortcuts. #566

merged 3 commits into from
Jun 26, 2017

Conversation

@SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Jun 23, 2017

This PR adds keyboard shortcuts to the editor

To test:

  • Run the test app on the iPad
  • On the editor tap on the CMD key
  • See if shorchuts show in screen
  • Try the shortcuts and see if they are working correctly.

@SergioEstevao SergioEstevao added this to the Version 1.0 Beta 5 milestone Jun 23, 2017
@SergioEstevao SergioEstevao requested a review from frosty June 23, 2017 13:07
frosty
frosty reviewed Jun 23, 2017
View reviewed changes
Example/Example/EditorDemoController.swift
return [ UIKeyCommand(input:"B", modifierFlags: .command, action:#selector(toggleBold), discoverabilityTitle:NSLocalizedString("Bold", comment: "Discoverability title for bold formatting keyboard shortcut.")),
UIKeyCommand(input:"I", modifierFlags: .command, action:#selector(toggleItalic), discoverabilityTitle:NSLocalizedString("Italic", comment: "Discoverability title for italic formatting keyboard shortcut.")),
UIKeyCommand(input:"D", modifierFlags: [.command, .alternate], action:#selector(toggleStrikethrough), discoverabilityTitle: NSLocalizedString("Strikethrough", comment:"Discoverability title for strikethrough formatting keyboard shortcut.")),
UIKeyCommand(input:"U", modifierFlags: .command, action:#selector(EditorDemoController.toggleUnderline(_:)), discoverabilityTitle: NSLocalizedString("Underline", comment:"Discoverability title for underline formatting keyboard shortcut.")),
Copy link
Contributor

@frosty frosty Jun 23, 2017

Choose a reason for hiding this comment

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

How come this one is namespaced EditorDemoController?

Copy link
Contributor Author

@SergioEstevao SergioEstevao Jun 23, 2017

Choose a reason for hiding this comment

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

Ask Swift :). It was getting confused with a method with the same name in super... This was the way to solve it :).

Copy link
Contributor

@frosty frosty Jun 23, 2017

Choose a reason for hiding this comment

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

Okay 👍

frosty
frosty reviewed Jun 23, 2017
View reviewed changes
Example/Example/EditorDemoController.swift Outdated
UIKeyCommand(input:"M", modifierFlags:[.command,.alternate], action:#selector(showImagePicker), discoverabilityTitle: NSLocalizedString("Insert Media", comment: "Discoverability title for insert media keyboard shortcut.")),
UIKeyCommand(input:"U", modifierFlags:[.command, .alternate], action:#selector(toggleUnorderedList), discoverabilityTitle:NSLocalizedString("Bullet List", comment: "Discoverability title for bullet list keyboard shortcut.")),
UIKeyCommand(input:"O", modifierFlags:[.command, .alternate], action:#selector(toggleOrderedList), discoverabilityTitle:NSLocalizedString("Numbered List", comment:"Discoverability title for numbered list keyboard shortcut.")),
UIKeyCommand(input:"H", modifierFlags:[.control, .shift], action:#selector(toggleEditingMode), discoverabilityTitle:NSLocalizedString("Toggle HTML Source ", comment: "Discoverability title for HTML keyboard shortcut."))
Copy link
Contributor

@frosty frosty Jun 23, 2017

Choose a reason for hiding this comment

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

In WPiOS, we currently use Cmd + Shift + H for HTML, not Ctrl. Is there a reason this is different?

Copy link
Contributor Author

@SergioEstevao SergioEstevao Jun 23, 2017

Choose a reason for hiding this comment

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

Ops, I hacked this in because in the simulator that is the shortcut for the Home Key, will change it now :)

frosty
frosty reviewed Jun 23, 2017
View reviewed changes
Example/Example/EditorDemoController.swift Outdated
UIKeyCommand(input:"H", modifierFlags:[.control, .shift], action:#selector(toggleEditingMode), discoverabilityTitle:NSLocalizedString("Toggle HTML Source ", comment: "Discoverability title for HTML keyboard shortcut."))
]
} else if htmlTextView.isFirstResponder {
return [UIKeyCommand(input:"H", modifierFlags:[.control, .shift], action:#selector(toggleEditingMode), discoverabilityTitle:NSLocalizedString("Toggle HTML Source ", comment: "Discoverability title for HTML keyboard shortcut."))
Copy link
Contributor

@frosty frosty Jun 23, 2017

Choose a reason for hiding this comment

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

Don't forget this one too :)

Copy link
Contributor Author

@SergioEstevao SergioEstevao Jun 23, 2017

Choose a reason for hiding this comment

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

It's done.

@frosty
Copy link
Contributor

frosty commented Jun 23, 2017

Looking good, Sergio – two more comments:

  • We should probably switch from using Cmd + Opt + D for strikethrough, as that brings up the Dock on iOS 11.
  • If you activate HTML mode with the keyboard shortcut, and the format bar isn't expanded, the only way to get out of HTML mode is the keyboard shortcut. Perhaps if you trigger it and the keyboard is not expanded, we should expand the keyboard. Alternatively, perhaps we should leave the toggle button enabled when in HTML mode.

@SergioEstevao
Copy link
Contributor Author

SergioEstevao commented Jun 23, 2017

@frosty

Cmd + Opt + S for strikethrough?

So can't they use the toolbar to switch back again?

@diegoreymendez
Copy link
Contributor

diegoreymendez commented Jun 23, 2017

If you activate HTML mode with the keyboard shortcut, and the format bar isn't expanded, the only way to get out of HTML mode is the keyboard shortcut.

That's more like a problem with the toolbar, though.

IMHO the HTML button should not be in the toolbar as it should be possible to switch regardless of if the caret is placed in the text view or not.

Either that, or the toolbar should at least be always visible, even if the caret is not in place.

Ping @iamthomasbishop ^

@SergioEstevao
Copy link
Contributor Author

SergioEstevao commented Jun 24, 2017

So for strikethrough with went for Cmd+S to be inline with Bold, Italic, Underline.

I also did some changes to make the toolbar expand when switching to HTML mode.

Mind to have another look @frosty ?

frosty
frosty approved these changes Jun 26, 2017
View reviewed changes
Copy link
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

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

Looks good! :shipit:

@SergioEstevao SergioEstevao merged commit c484c4e into develop Jun 26, 2017
@SergioEstevao SergioEstevao deleted the issue/add_keyboard_shortcuts branch June 26, 2017 11:28
@SergioEstevao SergioEstevao mentioned this pull request Jun 27, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@frosty frosty frosty approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Type] Enhancement

Projects

None yet

Milestone

Version 1.0 Beta 5

Development

Successfully merging this pull request may close these issues.

4 participants

@SergioEstevao @frosty @diegoreymendez