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

Pasting links in Gutenberg #21789

Open
amamujee opened this issue Apr 22, 2020 · 28 comments
Open

Pasting links in Gutenberg #21789

amamujee opened this issue Apr 22, 2020 · 28 comments
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.

Comments

@amamujee
Copy link

amamujee commented Apr 22, 2020

**Feature Request:**It would be cool if users had the option to select if pasting in a link is just a link vs. a link with some kind of preview. Here is a screen recording of how the twitter link pasting works on WP.com vs. link pasting.

Solution options:

  1. Link should not be pasted as text, but autolinked (should happen regardless)
  2. Give users the option to past as a link or as a preview
  3. A link pasted behaves like the Twitter embed

I think the middle option makes the most sense, and the first should happen regardless. I could see some value in making it a separate block if we wanted the experience to be 3)

https://d.pr/v/Xa2qSg

@mapk mapk added the Needs Design Needs design efforts. label Apr 22, 2020
@annezazu annezazu added [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement. and removed Needs Design Needs design efforts. labels Apr 22, 2020
@mapk
Copy link
Contributor

mapk commented Apr 22, 2020

We need some design around this. There are a couple issues related to this concept and it seems to become increasingly popular.

Related:
#15102
#18653

@paaljoachim
Copy link
Contributor

Associated:
#21029

PR:
#17413

@lancewillett
Copy link
Contributor

Related: #20442

Where the title from target page is created as the link text — like Google Docs.

@karmatosed
Copy link
Member

I'm going to dive into this and see how we can move it along a little. I am going to try and see if I understand the proposal first, to make sure. If I have got that wrong there are a lot of issues linked here, so I can course-correct with feedback.

Link should not be pasted as text, but autolinked (should happen regardless)

I agree with the idea that a link should auto-link by default, that just makes sense. The key raised here is choice, having the ability to choose either I think is where we can refine the flow. There seem 2 options here:

  • Add a step to ask if want to embed or just have text link.
  • Try and assume the best result, then offer an alternative.

I haven't included this option, but can explore if desired:

A link pasted behaves like the Twitter embed

My feeling was to embed all links could end up feeling very distracting.

Offering a step in between

5

This is a rough mockup just as a visual to get feedback on. In this, it would only do this if could embed, a link that was just a text-link wouldn't offer this.

Offering option to convert to text link only

Here you could have 2 optional locations, one in the toolbar (icon would need work) and one in the ellipsis menu.

Frame 2

Frame 1

Recommendation

My own feeling is avoiding the extra step probably is sensible in this case, but giving an option to change is good. Having it load then offering a modal or 'splash' that says 'continue or make as link' could also be another option, again though I wonder if it's adding a hurdle. Ultimately, that is a decision we need to take as to what is the default, but being able to adjust feels right.

Additional steps forward

The above is just one soothing of the experience. Let's do more by bringing in some additional tickets linked here.

Wrapping up into next steps

What would be great would be to get feedback on the above and then move the linked issues into development along with this one. I'm going to put the feedback label on so we can start that journey.

@karmatosed karmatosed added Needs Design Feedback Needs general design feedback. and removed Needs Design Needs design efforts. labels May 12, 2020
@paaljoachim
Copy link
Contributor

paaljoachim commented May 12, 2020

In Gutenberg 8.1 one can choose to transform any embed into a Paragraph Block. See this comment in the merged PR: #17413 (comment)

@mapk
Copy link
Contributor

mapk commented May 12, 2020

My own feeling is avoiding the extra step probably is sensible in this case, but giving an option to change is good.

I agree with this. Just as #17413 provides an option after the embed happens, a similar option can also work here as you've mocked up above.

Of the options, the most clearly understandable is the ellipses dropdown that you have with a "Convert to link" option. I'd suggest keeping it Sentence case and then converting the other options to Sentence case in another PR.

@karmatosed
Copy link
Member

@paaljoachim that's one option, for me though transforming feels a bit hidden, I don't know I guess you could argue that it's just as hidden in the ellipsis. I wonder though if you would associate paragraph with changing to a link?

@paaljoachim
Copy link
Contributor

paaljoachim commented May 12, 2020

Hey Mark and Tammie.
The other option which I believe @desaiuditd will work on next is this:
#21029 (comment)

As I see it any URL which wants to create a WordPress embed link should show a placeholder that asks the user if she/he wants to create an embed link or a text link. Similar to what I mention in the 21029 issue. If the user decides later to change from a WordPress embed then they can click the pencil and choose to convert to text link instead (as clicking the pencil the same placeholder screen will show up).

@desaiuditd
Copy link
Member

that's one option, for me though transforming feels a bit hidden, I don't know I guess you could argue that it's just as hidden in the ellipsis. I wonder though if you would associate paragraph with changing to a link?

@karmatosed
Just to be clear, are we thinking to keep both the options? That would be bad UX. Two ways of doing the same thing. Isn't it? Might be confusing and ambiguous to end-users.

@karmatosed
Copy link
Member

@desaiuditd I wouldn't suggest both, I would suggest just one and it's about deciding which one.

@paaljoachim
Copy link
Contributor

paaljoachim commented May 13, 2020

Thank you Tammie, for diving into this issue! Inspired by your thoughts as well as thoughts I have about this process here is my feedback.

A possible process of pasting a link.
Can the link be embedded.
No. Show the text link.
Yes. Show a placeholder screen.

1a
Add the placeholder right after the user has pasted the link that can be embedded.
(Possible update of the placeholder screen.)

New-Embed-Placeholder

The question that shows up.... is it intrusive to create the extra step in adding an Embed URL placeholder screen?

1b
An option: Embeds are automatically added. User clicks pencil to bring up the placeholder screen.

Screen Shot 2020-05-13 at 21 27 22

To change an embed to a text link the user can click the pencil to get access to the placeholder screen to where they could choose to convert the embed to a text link. (Current placeholder screen can be updated to give the option to convert to a text link.)

Screen Shot 2020-05-13 at 21 41 53

Another example with embedding a Youtube video.
Pasting in the Youtube URL automatically shows the embedded link.
Screen Shot 2020-05-13 at 21 36 29

Clicking the pencil/edit icon brings up the placeholder screen. (Here is the current placeholder which can then be updated with the option to change the embed to a text link.)
Screen Shot 2020-05-13 at 21 38 02

Bottom line:
Updating the existing placeholder screen making it possible to convert an embed to a text link would be very helpful. It uses an existing method and upgrades the possibility of what can be done in the placeholder screen.


2
Convert to link through the ellipsis menu. Can be done, but I believe we can easily begin to place too many options into this menu.

A familiar method is to use the visible pencil/or another edit icon to create an effect to the selected block.


So from thinking out loud through the above. To me it seems like the most logical approach would be to update the placeholder screen so the user can click the pencil in the toolbar of an embed to convert it into a text link.

@paaljoachim
Copy link
Contributor

I made an Embed prototype:
https://www.figma.com/proto/OfdH1LSa2MYLUt7iUDRhq7/Embed?node-id=1%3A2&scaling=min-zoom

It uses a placeholder and the 3 dot ellipsis menu.

@karmatosed
Copy link
Member

karmatosed commented May 21, 2020

I think it would be a good idea to focus in on the problem we're trying to solve and recapping where we are today. I appreciate the mocks, but I do think it's a good spot to check in on objective here. From there, absolutely it can move into iterations.

Currently, we have the option to transform as shown in #17413.

Next steps based on revising this flow would be to bring in the following:

  • Offer of choice on the placeholder
  • Option to have in block to convert

Now let's try and focus on each of those.

Placeholder

This would appear only if you paste a link which could embed. I think a simpler approach as I outlined here would be a good start for moving to code.

81696343-af2b3280-945b-11ea-8789-4fb6357a60cb

As far as embeds I think if identified as an embed we could go a step further and show what I outlined in #18653, however that I feel is an extra step. Including here to merge the issues:

81694902-cc5f0180-9459-11ea-8053-d58142927ea1

81694560-5f4b6c00-9459-11ea-9298-e6f8ad5f1338

Option to convert

There are a few proposed spots for this:

  • Transforms: merged
  • New button: adding new action into toolbar
  • Ellipsis
  • Hooking into pencil (edit)

I would suggest the ellipsis for me works best here and would like to see that over the transforms, which I am not totally convinced someone will think about. I could be persuaded for a transform and ellipsis, but I really am not sure this would be something you'd go there to discover. Again, happy to discuss that though.

Next steps

Whilst I appreciate the mocks I do think we can scale this back and see what the effect is on people. We don't need to dive right into a full scale dual button within editing or other options right now.

My feeling is we can based on the following move into development, but I would appreciate some feedback before as I am making some decisions here.

  1. We add a placeholder that appears if you paste a link that can also embed.
  2. We move transform to link to the ellipsis as show here:

81695220-3f687800-945a-11ea-9e36-19228c3c1079

I am closing #8653 so we can focus on this issue and get a solution. After that, it can be iterated. Let's see this as a step and then get feedback. Of course, if other mocks are wanted based on mine above that is totally ok, I do think we need to simplify though and see what this feels like first.

@paaljoachim
Copy link
Contributor

paaljoachim commented May 21, 2020

As you mentioned we have one way that is already merged which is to transform an embed from inside the standard Transform menu to a paragraph block (is not easy to be aware of but uses an existing method).

  1. Pasting a Youtube URL or any other link that automatically embeds, a placeholder shows up asking the question "Add embed" or "Add text link". It gives more control to how the user wants to show any type of link that automatically shows an embed.

I think the easiest way would be to add a general placeholder box for all embeds. Creating an embed box for each embed will likely be a lot of work making it too complex.

  1. Moving (not duplicating) the transform into the ellipsis menu seems to me as stepping away from a general usability method of how the Transform menu is used today. Blocks in general have a method to transform into other kinds of block through the existing Transform menu (even though it is a bit hidden).

@karmatosed
Copy link
Member

karmatosed commented May 21, 2020

I think the easiest way would be to add a general placeholder box for all embeds. Creating an embed box for each embed will likely be a lot of work making it too complex.

Just to be clear my note is to add a link and copy change for existing embeds, not to change the boxes for each placeholder, that then allows us to avoid a placeholder before the embed one.

The placeholder would happen if you just pasted a link.

@mapk
Copy link
Contributor

mapk commented May 22, 2020

I agree it's frustrating if I just want to paste in a link without an embed. But I feel this way because the option to convert it doesn't exist.

I'd like to provide the user a decisive solution and then offer options if they wish to change it.

This means that if we have an embed option, we just embed it. BUT we allow the user to convert it to a link using the ellipses menu as @karmatosed mocked up. Ultimately, I'd also like to see us allow the ability to paste without formatting, @ellatrix. Like maybe a Shift + CMD + V to get an unformatted paste. It wouldn't have it linked, but then the user can just make it a link from the toolbar.

What do y'all think if we started here? If you feel the placeholder is necessary when pasting a link that can be an embed, I'm open to that direction.

@paaljoachim
Copy link
Contributor

paaljoachim commented May 22, 2020

Thanks for sharing Mark @mapk !

I am open for the various ideas.
Placeholder.
Ellipsis menu.
Let's just leave the newly merged transform Embed to Paragraph block where it is. We can always add on additional methods. As each Block have their own transform menu options.

Having the Convert to text link option show up in the ellipsis menu when an embed is selected is very helpful. As the ellipsis menu will likely show relevant options depending on which block is selected. So the option would not be there on clicking blocks that are not embeds.

Sounds like a good move forward to add the option into the ellipsis menu.

@paaljoachim
Copy link
Contributor

@karmatosed
Can we ask @desaiuditd to begin development of adding a Convert to (text) link in the Ellipsis menu for Embeds?

@karmatosed
Copy link
Member

@desaiuditd that's fine, but please also remove the one for transforms, so we don't have both.

@karmatosed karmatosed added the Needs Dev Ready for, and needs developer efforts label May 26, 2020
@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label May 26, 2020
@paaljoachim
Copy link
Contributor

paaljoachim commented May 26, 2020

Hey Tammie. In regards to two methods of transforming an embed. One in the Transform to panel next to Group. I see no harm in keeping the already merged transform to Paragraph in the Transform to panel. As it gives the user two different methods to convert an embed. Users will choose different ways. The Transfom to panel is meant as a place to transform the selected block to some other block. Transforming the Embed to a Paragraph block also belongs in the specific panel.

Screen Shot 2020-05-26 at 16 58 16

Having the "Convert to text link" in the Ellipsis menu. Might be easier to find for some.

Convert-to-Text-link

In general lets add blocks to the Transform to panel. Giving the option for users to in general use this panel to transform any kind of block.

@mapk
Copy link
Contributor

mapk commented May 26, 2020

Initially when I read about using the Transform tool to transform the embed to a link, I wasn't sure how that would happen. A "link" isn't really a block. But a Paragraph is! Thanks for the visual, @paaljoachim. This makes a bit more sense to me (but I know Gutenberg), does it make sense to a user? Maybe? Either way, it's probably okay allowing the block to be transformed to a Paragraph regardless because it's utilizing an existing Gutenberg ability.

@desaiuditd
Copy link
Member

@desaiuditd that's fine, but please also remove the one for transforms, so we don't have both.

Cool. I'll try to work on this in this week. Will raise a PR.

@karmatosed
Copy link
Member

Thanks @desaiuditd that would be great. Let's remove transforms and take it from there as far as direction. If it does feel we need more methods then we absolutely can iterate. We need to distil back first though.

@desaiuditd
Copy link
Member

desaiuditd commented May 30, 2020

I started working on this and I saw that Embed block code is written in good ol' React Class based component. And I'm like ... no, I'm not touching that. 😉 On serious note, I need to be able to use certain React hooks e.g., useSelect, useDispatch etc. I could use withSelect and withDispatch in existing structure.

But then I remembered @mcsf 's comment on #17413 that

The embeds code base could use an important refresh

So I thought this could be a good opportunity to refactor the Embed block code. At least, converting the edit component into a function component to start with. So that React Hooks can be used within.

import { Component } from '@wordpress/element';
export function getEmbedEditComponent( title, icon, responsive = true ) {
return class extends Component {
constructor() {
super( ...arguments );

But before I do that, can I get consent from someone senior from the dev community? Any thoughts/opinions?

Of course, I can do that in separate PR, to maintain separation of concerns. One, just for the refactor and second, to actually add the Convert to Link option.

Update: I've got a branch ready which refactors the Embed edit component. Let me know if that's cool and I can raise a PR. Thanks. 🙂

@paaljoachim
Copy link
Contributor

Hey Udit @desaiuditd

This would be something we can bring up during the Core Editor meeting on Wednesday. As it is a good place to create some discussions around refactoring the Embed block code.

@ZebulanStanphill
Copy link
Member

Our coding guidelines recommend the usage of function components, so I'd say this is a perfect opportunity to refactor the Embed block to use hooks.

When possible, all components should be implemented as function components, using hooks for managing component lifecycle and state.

@desaiuditd
Copy link
Member

Great! I’ll open PR soon!

@jasmussen
Copy link
Contributor

#27915 has some valuable additions to this issue, worth a look.

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) Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

9 participants