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.

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

Add unlink button to escape out of adding link #1146

Merged
merged 5 commits into from
Jun 16, 2017
Merged

Conversation

mkaz
Copy link
Member

@mkaz mkaz commented Jun 12, 2017

Add ability to cancel adding a link after you clicked the linked button.
Fixes #778

@mkaz
Copy link
Member Author

mkaz commented Jun 12, 2017

@jasmussen - first part is adding the extra unlink button when adding the link is clicked. Screenshot below, can I get your design eyes and let me know what you think.

add-unlink-button

I'm trying now if I can grab the Esc key to cancel just adding the link and not kill the whole text block

@mkaz
Copy link
Member Author

mkaz commented Jun 12, 2017

@notnownikki I think I have this working as you described the bug. A cancel button was added and the ability to escape when it opens without canceling the full block. Take a look and let me know what you think.

@mkaz mkaz self-assigned this Jun 12, 2017
@jasmussen
Copy link
Contributor

UX seems nice! 👍

Copy link
Member

@notnownikki notnownikki left a comment

Choose a reason for hiding this comment

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

This doesn't work for me...

This is what I did:

I added a new text block and typed some text. I selected the text I wanted to turn into a link. I entered "http://google.com/" into the link box and pressed enter.

I expected the input to disappear at that point so I can carry on with the post, but it doesn't. Clicking back in the block doesn't make it go away either. So I pressed escape to get out of the link editing, and it removed the link completely.

I tried the same steps again, but tried clicking the "enter" icon instead, and that seemed to make the link but still left me in the input UI. I had to add another block, focus the new block, and go back to the original block to get out of it. When I did that, the text looked like a link, but clicking on it to bring back the link editing UI showed that there was no URL in the link.

}

componentDidMount() {
document.addEventListener( 'keydown', this.onKeyDown );
Copy link
Member

Choose a reason for hiding this comment

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

If keydown the event we want, or do we want to wait for a complete press?

@@ -44,6 +50,19 @@ class FormatToolbar extends wp.element.Component {
}
}

onKeyDown( keydown ) {
switch ( keydown.keyCode ) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a simple if would be enough here for now.

@@ -44,6 +50,19 @@ class FormatToolbar extends wp.element.Component {
}
}

onKeyDown( keydown ) {
Copy link
Member

Choose a reason for hiding this comment

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

By convention we might want to name this variable event.

mkaz added 5 commits June 16, 2017 07:04
Add a keydown listener on component, which if you hit Escape while you
are editing the link will stop without canceling out of the block. If
you hit escape again, it will cancel the block.
@mkaz mkaz force-pushed the bug/778-abort-link branch from 0e01839 to 1882f05 Compare June 16, 2017 14:11
@mkaz
Copy link
Member Author

mkaz commented Jun 16, 2017

@notnownikki - there is an extra step after hitting (or clicking) enter that converts what was entered into a link and you can see it as a link (blue underline) in the input field. You then need to type Esc, or click out and it closes for me. So I'm not duplicating what it sounds like you described.

The change I added was not to fix the above behavior, which seems to be working as designed. But to fix if you start a link and don't type anything and then hit escape to cancel making an empty link.

@notnownikki
Copy link
Member

Ah, I see now, the problems I'm seeing are caused by selection code, totally unrelated to this branch. Somehow I'm able to lock things up by the way I select the text to convert to a link, and end up with a URL input box that I can't get rid of.

But the ESC functionality works :)

@notnownikki notnownikki dismissed their stale review June 16, 2017 14:36

Bugs were unrelated to the changes in this branch

@mkaz mkaz merged commit a9b0950 into master Jun 16, 2017
@mkaz mkaz deleted the bug/778-abort-link branch June 16, 2017 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants