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

Block Editor: LinkControl: Present all options while editing link #20007

Closed
aduth opened this issue Feb 3, 2020 · 16 comments · Fixed by #20052
Closed

Block Editor: LinkControl: Present all options while editing link #20007

aduth opened this issue Feb 3, 2020 · 16 comments · Fixed by #20052
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Needs design efforts. [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented Feb 3, 2020

Previously: #19462 (comment)
Related: #19365, #19570, #19971

A new link component introduced for the purposes of supporting the Navigation Link block (#17557, #18061) was incorporated into the paragraph block in #19462. The goal of this was to ensure a consistent link editing experience for all parts of the editor. However, the new link component regresses in the ideal workflow for editing a link.

From @MarcoZehe at #19462 (comment):

So, I want to select text, press CTRL+K or CMD+K, paste the URL, enter to submit, or tab to the submit button and hit Enter or Space, and be back in the text to continue working.

Because the revised link component only shows the "Open in New Tab" button as part of its preview state, concessions were made in #19462 such that pressing CMD+K while having a selection of text will only shift focus into the popover, but the user must still tab and select the "Edit" button to change the URL.

Proposal: The "Open in New Tab" toggle should be presented as an optional part of the editing state of the link component.

After #19365 Before #19365 Classic Editor
Link editing after #19365 image Link editing in Classic Editor

In the screenshots above, you can see that all link editing prior to #19365 would allow access to change settings associated with the link, including "Open in New Tab".

Important to consider are two related issues:

Implementation proposal:

  • When editing a link, the input should be accompanied by "Submit" and "Advanced Settings" buttons.
  • Clicking "Advanced Settings" should toggle the display of the "Open in New Tab" toggle option, as it did previously (needs design).

I am not certain one way or another whether "Open in New Tab" should apply the changes immediately when in the mode of editing a link. Considering #19365, I believe that part of the problem might have been attributable to the fact that there was very little visual difference between the "Preview" and "Editing" states of this dialog, and thus the user may have been confused about whether they were editing or previewing the link. This is less problematic with the new link UI, since the states are more visually distinctive. Otherwise, the expectations of editing a link could be such that the user would only expect their changes to take effect once submitted.

  Viewing Editing
Previous UI Previous UI viewing Previous UI editing
New UI New UI viewing New UI editing
@aduth aduth added [Type] Task Issues or PRs that have been broken down into an individual action to take [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Needs design efforts. labels Feb 3, 2020
@jasmussen
Copy link
Contributor

jasmussen commented Feb 4, 2020

Thanks for working on this. A unified URL dialog has great value so it's the same interface to learn across links, navigation blocks, and possibly the Social Links block per some ideas @mcsf are exploring.

In my understanding, the biggest challenge of the new UI is that the on-selected-link popover is not easily accessible via keyboard, and this is now the only way to make a link able to be opened externally.

We could take additional inspiration from Google Docs, where you create a link like so:

  • Select text, press ⌘K
  • Paste link
  • Press Enter with focus in the link field, or tab to the Apply button and press Enter

create link

Google Docs also has an on-select link popover:

  • Click it to open it
  • Press ⌘K with a link selected to go directly to editing it

link popover

The Google Docs link popover does not contain any features that are not accessible via other keyboard shortcuts, making effectively a duplicate UI/affordance for mouse users.

One basic idea is for us to do the same:

  • Select text, press ⌘K
  • Paste link
  • Press Enter with focus in the link field
  • ... or tab to the Apply/Submit button and press Enter
  • ... or tab to the "Open in external window" toggle, or "advanced settings" toggle, and press Enter to toggle those, then shift tab back to the Submit button.

We would also want to make it so when the link popover is showing, ⌘K works to edit that link. But in doing so, the link popover would be come duplicate, not essential, UI.

Here's a design that would work for the link popover. It's not quite as bare as #19971, but it should nonetheless scale across links and navigation menu items and even social links:

Screenshot 2020-02-04 at 10 16 04

Specifically for hyperlinks, we could take it a step further and include the editable text:

Screenshot 2020-02-04 at 10 16 08

Note that those mockups are made in the style of #18667, but the general UI components should scale just fine to the existing components.

@aduth
Copy link
Member Author

aduth commented Feb 4, 2020

As always, thanks for the thorough exploration @jasmussen ! It aligns with how I understand the problem and its solutions.

We would also want to make it so when the link popover is showing, ⌘K works to edit that link. But in doing so, the link popover would be come duplicate, not essential, UI.

For me, this is an important takeaway from your comment. A problem with the current implementation is that we've placed essential interactions into the link preview dialog, forcing us to accommodate some means to reach those as a distinct part of the link editing experience.

... or tab to the "Open in external window" toggle, or "advanced settings" toggle, and press Enter to toggle those, then shift tab back to the Submit button.

To clarify, would you suggest that "Open in New Tab" still require the confirmation step of submitting the changes? (Touching on #19365 and related points in the original comment)

Would it still make sense to show "Open in New Tab" in the preview state of the link? This existed prior to #19365 as well (under a dropdown), but I think it may have compounded the problem of #19365 to have the toggle exist in multiple places, where in one place it would take effect immediately, and in the other only when submitting all changes. For me at least, I think it would be nice to consolidate this into the conscious choice to edit the link. It also doesn't seem like it's such a common workflow that it demands placement in the preview (especially when that placement is otherwise not accessible by keyboard).


(cc @shaunandrews @getdave for additional thoughts and feedback)

@jasmussen
Copy link
Contributor

To clarify, would you suggest that "Open in New Tab" still require the confirmation step of submitting the changes? (Touching on #19365 and related points in the original comment)

There's some nuance I missed here, apologies. Is this mostly a technical consideration? I.e. flipping the toggle would trigger an auto-save/save refresh? Definitely think that the change should be remembered once set.

Would it still make sense to show "Open in New Tab" in the preview state of the link?

If it's duplicate UI, it doesn't need to be there (so maybe remove?) but I also don't have a strong opinion that it can't be if it solves some other problem. Personally I find those little link popovers helpful primarily to allow you to follow the link in question (like middle-click to open in new tab).

@aduth
Copy link
Member Author

aduth commented Feb 4, 2020

To clarify, would you suggest that "Open in New Tab" still require the confirmation step of submitting the changes? (Touching on #19365 and related points in the original comment)

There's some nuance I missed here, apologies. Is this mostly a technical consideration? I.e. flipping the toggle would trigger an auto-save/save refresh? Definitely think that the change should be remembered once set.

Not so much technical as much as: Should any edits to the link be applied immediately upon change, or must they be submitted?

Consider:

  • When you start typing a URL, we don't change the HTML of the paragraph for each keystroke of the URL, we only change it after you have finished (and confirmed) your changes
  • In the Classic Editor, if you toggled "Open in new tab" in the dialog and then clicked off-screen (to dismiss the dialog), the changes wouldn't be saved.

Classic editor edit link dialog

The latter does a better job of illustrating the point of this being a form that only takes effect once submitted. The problem we face is that our link editing UI doesn't really look like a traditional form, nor would I so closely associate "clicking away" to be as much of a "Cancel" intent as it would be when it's presented like a modal in the Classic Editor.

@jasmussen
Copy link
Contributor

jasmussen commented Feb 4, 2020

Ah, thank you for clarifying.

My first instinct was to only have link properties save once you "apply" the link, confirm the dialog. But then I remembered that the new tab option uses a toggle/switch control, and such a control should only be used when paired with instant effect (like a light switch), so in this case, saving.

So if it's easiest to save the link on confirm, just change the new tab option to use a checkbox instead of a switch.

@aduth
Copy link
Member Author

aduth commented Feb 4, 2020

I don't think one or the other would be any more technically challenging to implement. Based on what you're saying, I'd tend to agree with you that, if it is to remain as a ToggleControl (vs. a checkbox), then it should take effect immediately.

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

So if it's easiest to save the link on confirm, just change the new tab option to use a checkbox instead of a switch.

There is actually a challenge to consider, at least in the practical implications of what it means to "change" a value for this component. As part of the workflow where submitting a change to a link in a paragraph should move the selection back to the paragraph, the way this is implemented is to focus the paragraph when the value changes (source). The problem is: If toggling "Open in New Tab" is considered a change, this would cause focus to shift back to the paragraph when toggled, which is not desirable.

There are a few solutions I can imagine:

  • Value changes can only occur when submitted (i.e. change the toggle to a checkbox, and require the user to submit the toggled change)
  • Or: LinkControl exposes some way to distinguish between changes of these settings vs. submission of the new link (e.g. extra parameter on onChange, or a new prop onSubmit or onIsEditingToggled)
  • Or: Separate these settings properties out of the LinkControl value (some related discussion at How we can approach the introduction of a new link component within Gutenberg #18061 (comment))
  • Or: Update the inline link formatting implementation to accommodate (e.g. only shift focus if the url changes, or some other way to determine that a form submission occurred)

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

Here's a design that would work for the link popover. It's not quite as bare as #19971, but it should nonetheless scale across links and navigation menu items and even social links:

Screenshot 2020-02-04 at 10 16 04

Noting that the specific verbiage here would either need to be made more generic (e.g. "Apply"), or otherwise we need to add support to LinkControl to distinguish between links being edited or added. As proposed, it might be confusing if the user is prompted to "Add" when in fact they are editing the URL of an existing link.

Edit: A variation on "made more generic" can be to return to using an icon, i.e. submit icon. Personally I prefer the written form, as it is consistent with the "Preview" state showing an "Edit" label, and may be more accessible / usable. But I also grant that if a generic label of "Apply" is as much or more ambiguous than an equivalent icon, it might be the preferable option.

Edit 2: Depending if a choice is made to require explicit submission of "Open in New Tab" toggle, a label of "Apply" could help to lend clarity that a submission is required for the changes to take effect.

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

Specifically for hyperlinks, we could take it a step further and include the editable text:

Screenshot 2020-02-04 at 10 16 08

Noting that implementing search results in this style would be (and is) made difficult by the fact that suggestions are rendered as part of the URLInput, while the buttons / expand toggle are not part of this component. Right now we deal with this using CSS to absolutely position these buttons atop of the input, but this is rather fragile (see also #19971).

Solutions may be:

Pseudo-code for the third solution:

const urlInput = useRef();

return (
    <form>
        <URLInput ref={ urlInput } value="http://example.com" />
        <Button type="submit" />
        <URLInput.Suggestions inputRef={ urlInput.current } />
    </form>
);

Edit: Another option is to change the design to accommodate the fact that URL input and its suggestions are rendered together and distinctly. For example, Google Doc's link search behaves like this:

image

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

I've published a very early draft pull request at #20052 to try to start the ball rolling with these changes, in hopes we can try to remedy this workflow before next week's 5.4 beta.

@jasmussen
Copy link
Contributor

There is actually a challenge to consider, at least in the practical implications of what it means to "change" a value for this component.

and

Value changes can only occur when submitted (i.e. change the toggle to a checkbox, and require the user to submit the toggled change)

Let's picture the checkbox version — you select text, press ⌘K, check the box, and close the link dialog. As soon as this happens, the post is marked "dirty" in need of a save. This seems sufficient, would it be doable?

@jasmussen
Copy link
Contributor

Noting that the specific verbiage here would either need to be made more generic (e.g. "Apply"), or otherwise we need to add support to LinkControl to distinguish between links being edited or added. As proposed, it might be confusing if the user is prompted to "Add" when in fact they are editing the URL of an existing link.

A different label is totally fine especially for starters, we can always add complexity if we find a need to do so! Apply is fine, for example.

Noting that implementing search results in this style would be (and is) made difficult by the fact that suggestions are rendered as part of the URLInput, while the buttons / expand toggle are not part of this component. Right now we deal with this using CSS to absolutely position these buttons atop of the input, but this is rather fragile (see also #19971).

Yep, definitely no need to adhere too closely to the mockups and make it perfect — consider it just a mockup for the futur UI components, but definitely leverage the design and what we have today I think I should be able to make the two look disconnected, but that's definitely not necessary for V1.

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

Let's picture the checkbox version — you select text, press ⌘K, check the box, and close the link dialog. As soon as this happens, the post is marked "dirty" in need of a save. This seems sufficient, would it be doable?

Assuming that by "close the link dialog", you mean to click away or press Escape (cancel), my expectation would be different. Following these steps, I would expect to not be prompted to save the post, because I never actually applied the changes to the paragraph block. I see the potential confusion here, due to how this is something like a form inside a form (link editor inside the post editor). It worked this way in the Classic editor as well, but like illustrated by the screenshot in #20007 (comment), the previous "Insert/edit link" was so much more clearly a separate form (in the modal) that I think it benefitted the clarity that clicking away or dismissing the modal would not have changed the post in any way.

@jasmussen
Copy link
Contributor

Assuming that by "close the link dialog", you mean to click away or press Escape (cancel), my expectation would be different. Following these steps, I would expect to not be prompted to save the post, because I never actually applied the changes to the paragraph block.

Correct, thanks for noting my inaccurate language. Cancelling the action should not mark it dirty, but applying ... could, right?

I'll take a look at your PR tomorrow btw!

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

Correct, thanks for noting my inaccurate language. Cancelling the action should not mark it dirty, but applying ... could, right?

Yes, that's right. The paragraph (and consequently the post) would be changed if -- and only if -- the user clicks "Apply". #20052, despite being in an otherwise messy state, does currently incorporate all of this.

@jasmussen
Copy link
Contributor

Awesome. If that feels right to you, it seems like a perfectly okay solution. Like I said I'll also sanity check tomorrow.

@aduth aduth added the [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) label Feb 10, 2020
jasmussen pushed a commit that referenced this issue Feb 12, 2020
This will no doubt need additional feedback, and the branch should stay open for a while to gather all those thoughts.

One of the primary reasons for reducing the borders is, similar to tertiary buttons, that they work due to their context, just like how menu ite
ms work in their menu item contexts. This puts a great onus on the implementer to choose the right button for the right context. In that vein, w
e may even want to provide additional buttons, such as a solid background button that isn't primary, for cases where a primary button is insuffi
cient, and a secondary button does not work well enough in the context. An example could be considered the link dialog mocked up here #20007 (comment)
jasmussen pushed a commit that referenced this issue Feb 18, 2020
This will no doubt need additional feedback, and the branch should stay open for a while to gather all those thoughts.

One of the primary reasons for reducing the borders is, similar to tertiary buttons, that they work due to their context, just like how menu ite
ms work in their menu item contexts. This puts a great onus on the implementer to choose the right button for the right context. In that vein, w
e may even want to provide additional buttons, such as a solid background button that isn't primary, for cases where a primary button is insuffi
cient, and a secondary button does not work well enough in the context. An example could be considered the link dialog mocked up here #20007 (comment)
jasmussen pushed a commit that referenced this issue Feb 19, 2020
This will no doubt need additional feedback, and the branch should stay open for a while to gather all those thoughts.

One of the primary reasons for reducing the borders is, similar to tertiary buttons, that they work due to their context, just like how menu ite
ms work in their menu item contexts. This puts a great onus on the implementer to choose the right button for the right context. In that vein, w
e may even want to provide additional buttons, such as a solid background button that isn't primary, for cases where a primary button is insuffi
cient, and a secondary button does not work well enough in the context. An example could be considered the link dialog mocked up here #20007 (comment)
jasmussen pushed a commit that referenced this issue Feb 20, 2020
This will no doubt need additional feedback, and the branch should stay open for a while to gather all those thoughts.

One of the primary reasons for reducing the borders is, similar to tertiary buttons, that they work due to their context, just like how menu ite
ms work in their menu item contexts. This puts a great onus on the implementer to choose the right button for the right context. In that vein, w
e may even want to provide additional buttons, such as a solid background button that isn't primary, for cases where a primary button is insuffi
cient, and a secondary button does not work well enough in the context. An example could be considered the link dialog mocked up here #20007 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Needs design efforts. [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants