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

Insert link warning (button block) and usability - consistency #1670

Closed
afercia opened this issue Jul 3, 2017 · 10 comments
Closed

Insert link warning (button block) and usability - consistency #1670

afercia opened this issue Jul 3, 2017 · 10 comments
Labels
[Feature] Blocks Overall functionality of blocks [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback.

Comments

@afercia
Copy link
Contributor

afercia commented Jul 3, 2017

Not sure what are the plans for the insert link "inline toolbar" or if this is already on the radar, so please close this issue if it's pointless.

Noticed two issues:

Button block: inserting the URL triggers a React warning:
As soon as I type the first character in the URL field:

screen shot 2017-07-03 at 16 48 44

Warning: A component is changing an uncontrolled input of type url to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component.
More info: https://fb.me/react-controlled-components
in input (created by edit)

Looks similar to #680

Usability/consistency:
When inserting a link on some selected text, the inline toolbar "Apply" button changes to an "Edit" button. The URL displayed in the toolbar is clickable and points to the linked resource, and clicking on "Edit" makes the URL editable again:

screen shot 2017-07-03 at 16 18 37

Instead, on the button block the behavior is different: after a link is inserted, the "Apply" button doesn't change. There's no "Edit" button and the URL displayed in the toolbar is always editable:

screen shot 2017-07-03 at 16 43 29

Not sure if there are technical reasons for this. ideally, the behavior should be similar. I can understand the button block doesn't necessarily need a "Remove link" button but I'd say the insert/edit user experience should be as similar as possible with the one of standard links.

@ellatrix
Copy link
Member

ellatrix commented Jul 3, 2017

+1 You should also be able to navigate to the page. :)

@afercia
Copy link
Contributor Author

afercia commented Jul 3, 2017

Yep! Also, editing an already inserted link with keyboard only is a bit hard right now. I guess that will improve with the introduction of a Cmd/Ctrl+K shortcut together with the other planned shortcuts, see #71

@mtias mtias added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Blocks Overall functionality of blocks labels Aug 31, 2017
@mtias mtias modified the milestones: Beta 1.2, Beta, Needs to happen Aug 31, 2017
@jorgefilipecosta jorgefilipecosta self-assigned this Oct 19, 2017
@mtias
Copy link
Member

mtias commented Nov 20, 2017

We have changed how the button urlInput looks, but it should also be more consistent functionality wise. Feel free to reopen if there are still concerns.

@mtias mtias closed this as completed Nov 20, 2017
@afercia
Copy link
Contributor Author

afercia commented Nov 20, 2017

@mtias sorry maybe I'm missing something but I don't see relevant changes here. The button link inline toolbar and the normal link inline toolbar still work in a different way and would benefit from some better consistency.

Also, the JS warning is still there:
A component is changing an uncontrolled input of type text to be controlled.

@afercia afercia reopened this Nov 20, 2017
@mtias mtias added the Needs Design Feedback Needs general design feedback. label Mar 8, 2018
@jeffpaul
Copy link
Member

This ticket was mentioned in Slack in #core-editor by jeffpaul. View the logs.

@jorgefilipecosta jorgefilipecosta removed their assignment Apr 4, 2018
@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Accessibility Apr 12, 2018
@karmatosed
Copy link
Member

Is this still something to consider in light of the changed to the link interface?

@karmatosed karmatosed removed this from the Merge Proposal: Accessibility milestone Apr 13, 2018
@afercia
Copy link
Contributor Author

afercia commented Apr 13, 2018

@karmatosed I see there are still at least 3 different types of UI to insert a link, see the ones on the blocks. I don't see a reason why they should be different, unless I'm missing something.

cover image:

cover block insert link UI

image:

image block insert link UI

paragraph (partially hidden by the following image block):

paragraph block insert link UI

button

button block insert link UI

@karmatosed
Copy link
Member

karmatosed commented Apr 15, 2018

@afercia I totally agree in unifying them to the new UI and we can using this as a starting point: #6107

@karmatosed
Copy link
Member

Unless I am wrong we should close this in light of the issue above. We can always reopen but lets make sure we get all the link interfaces unified there.

@afercia
Copy link
Contributor Author

afercia commented Apr 28, 2018

Sure. For reference: #6392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

No branches or pull requests

6 participants