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

LinkControl: Open popup with cmd+k shortcut #18405

Merged
merged 1 commit into from
Nov 8, 2019
Merged

LinkControl: Open popup with cmd+k shortcut #18405

merged 1 commit into from
Nov 8, 2019

Conversation

WunderBart
Copy link
Member

@WunderBart WunderBart commented Nov 8, 2019 •
edited
Loading

Description

Added the cmd/ctrl+k shortcut handling to the LinkControl popup. It should work the same way as it has in the RichText block.

Screenshots

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

Closes:

@getdave getdave added the [Status] In Progress Tracking issues with work in progress label Nov 8, 2019
@getdave getdave added this to 👀 PRs to review in Navigation editor via automation Nov 8, 2019
@getdave getdave added the [Package] Block editor /packages/block-editor label Nov 8, 2019
getdave
getdave approved these changes Nov 8, 2019
View reviewed changes
Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

Implementation looks solid.

I wasn't sure about bindGlobal but I read the docs and it's required if you're adding a shortcut that you want to work whilst a text field has focus so this is legit.

Great job on adding the shortcut prop to ToolbarButton as well 👍

@getdave
Copy link
Contributor

getdave commented Nov 8, 2019

Looks like Build had failed so I triggered a re-run for you.

obenland
obenland approved these changes Nov 8, 2019
View reviewed changes
Copy link
Member

@obenland obenland left a comment

Choose a reason for hiding this comment

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

Nice work Bart

retrofox
retrofox approved these changes Nov 8, 2019
View reviewed changes
Copy link
Contributor

@retrofox retrofox left a comment

Choose a reason for hiding this comment

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

LGTM! nice one \m/

@getdave getdave merged commit c721a4d into WordPress:master Nov 8, 2019
Navigation editor automation moved this from 👀 PRs to review to ✅ Done Nov 8, 2019
@WunderBart WunderBart deleted the add/link-control-keyboard-shortcut branch November 8, 2019 17:21
@youknowriad youknowriad removed the [Status] In Progress Tracking issues with work in progress label Nov 11, 2019
@youknowriad youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019
@retrofox retrofox mentioned this pull request Nov 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@retrofox retrofox retrofox approved these changes

@getdave getdave getdave approved these changes

@obenland obenland obenland approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

Assignees

@WunderBart WunderBart

Labels
[Package] Block editor /packages/block-editor
Projects
No open projects
Navigation editor
  
Done
Milestone
Gutenberg 6.9
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@WunderBart @getdave @retrofox @obenland @youknowriad