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

Bug: Link sometimes embeds other times not. #21029

Closed
paaljoachim opened this issue Mar 19, 2020 · 16 comments · Fixed by #25370
Closed

Bug: Link sometimes embeds other times not. #21029

paaljoachim opened this issue Mar 19, 2020 · 16 comments · Fixed by #25370
Assignees
Labels
[Block] Embed Affects the Embed Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 19, 2020

WordPress 5.3.2
Using Gutenberg version 7.7.1. (same affect when deactivating the Gutenberg plugin.)
Twenty Twenty theme.

Describe the bug
Adding a link through pasting a link into a paragraph block sometimes makes it into an embedded link and other times a text link.

To reproduce
Steps to reproduce the behavior:

  1. In a paragraph block paste this link: https://make.wordpress.org/
  2. Result should be an embedded link.

Screen Shot 2020-03-19 at 21 40 04


  1. In a paragraph block paste this link: https://make.wordpress.org/core/
  2. An Embed URL dialog box shows up saying "Sorry, this content could not be embedded."

Screen Shot 2020-03-19 at 21 41 06

I click the Convert to (text) link.
Screen Shot 2020-03-19 at 21 42 52

The result on the frontend showing an embed and a text link:
Screen Shot 2020-03-19 at 21 43 54

Here is an example where I also pasted the following link into a paragraph block: https://make.wordpress.org/test/2020/03/11/gutenberg-usability-testing-for-january-2020/
Frontend result showing three different visuals.

Screen Shot 2020-03-19 at 21 53 32

Expected behavior
I expect to be able to choose when or if I want a text link to be embedded.

If a link wants to embed, then I expect a dialog screen to show up similar to this mockup.
Embed-link-option-Gutenberg

Desktop (please complete the following information):

  • OSX 10.13.6
  • Browser Chrome
  • Gutenberg version 7.7.1

Plugin support issues:
https://wordpress.org/support/topic/paste-links-in-gutenberg-without-embedding/
https://wordpress.org/support/topic/paste-link-without-creating-an-embed-block/

@mapk
Copy link
Contributor

mapk commented Mar 19, 2020

This makes sense to me. It sounds here like Gutenberg may be trying to be too clever.

If Gutenberg is going to convert the link into an embed, showing the embed block placeholder feels like a good solution.

@paaljoachim
Copy link
Contributor Author

paaljoachim commented Mar 19, 2020

Same test as above.
Theme Twenty Twenty.
WordPress: 5.4-RC3-47470
Deactivated Gutenberg plugin.

Backend:
Screen Shot 2020-03-20 at 00 51 04

Frontend:
Screen Shot 2020-03-20 at 00 51 34

Again three different versions of how a link is seen.

@mcsf
Copy link
Contributor

mcsf commented Mar 23, 2020

Hi. I believe there are a couple of issues to discuss here:

  1. I don't believe that, strictly speaking, the bug consists of Gutenberg sometimes embedding and sometimes not. It always tries to embed, but this process may fail depending on the source site.
  2. However, we could file an enhancement request so that Gutenberg gives the user more choice. Whether that means always showing the placeholder with the option to embed or convert to link, or allowing embeds to be "undone" or even transformed to Paragraph — that's another design decision.
  3. One thing that we really should do, IMO, is make sure the WP embeds in the editor look like the final result in the front end. This seems to be specific to WordPress-type embeds. I haven't looked under the hood, so I don't know if it's just a style issue or if there's more to it. To me that's the only proper bug in this scenario. :)

@mcsf
Copy link
Contributor

mcsf commented Mar 23, 2020

One thing that we really should do, IMO, is make sure the WP embeds in the editor look like the final result in the front end. This seems to be specific to WordPress-type embeds. I haven't looked under the hood, so I don't know if it's just a style issue or if there's more to it. To me that's the only proper bug in this scenario. :)

Upon testing, this seems to be a regression between WP 5.2 and WP 5.3.

@joaquimds
Copy link

joaquimds commented Mar 31, 2020

Hi all, my issue is linked to from this but is slightly different - I would like Gutenberg to fallback to creating a regular hyperlink if the embed block is disabled. This only fails if the link is the only content on the line, but it is still causing an issue for one of the sites I am developing. This issue has been reported before here: https://wordpress.org/support/topic/paste-links-in-gutenberg-without-embedding/

I am happy to try to fix this issue myself - I know WordPress fairly well as an end-user and I have made my own custom blocks - but I could do with being pointed in the right direction. Thanks!

@mcsf
Copy link
Contributor

mcsf commented Apr 3, 2020

I would like Gutenberg to fallback to creating a regular hyperlink if the embed block is disabled

This is a reasonable fallback. Just to be sure, how are you disabling the Embed block?

@desaiuditd
Copy link
Member

2. or allowing embeds to be "undone" or even transformed to Paragraph

This could be fixed with #17413. I've just updated that PR. Review would be helpful.

Interestingly, for @joaquimds 's usecase, after disabling the Embed from the Block Manager, I tried pasting the link in the editor, it failed to put the content in the paragraph block. The paragraph block stayed empty. However, it could be treated as a separate bug (Happens in v7.8.1 as well).

@desaiuditd
Copy link
Member

Expected behavior
I expect to be able to choose when or if I want a text link to be embedded.

With #17413, you can convert the embed back to paragraph with a link.

@paaljoachim
Copy link
Contributor Author

paaljoachim commented Apr 5, 2020

My suggestion would be to have all pasted links by default show up as regular text links. The user can choose to transform from a text link to an embed when there is a transform option available to do so.

(There would be an embed icon in the transform panel when there is an option to transform a link to an embed.)
Screen Shot 2020-04-05 at 12 12 53

As all links can be seen as text links.
Only some links can be seen as embedded links.

Use case.
User has 10 links to paste into a post/page. Today some would show up as embedded blocks while others as text links. Mixing embed and not embedded links.
Having all links show up as default text paragraph links would give a balanced list of links.

Embed is a transform the user should opt into changing a link to.

EDIT:
Option 2.
Pasting a link when it is not embeddable automatically adds it as a text link.
Pasting a link when it is embeddable opens a placeholder box to where the user can choose to embed or choose to paste it as a text link. (There also needs to be a simple way to revert to either text or embed link.)

Use case.
User pasts a link from another WordPress site. He/She knows that one can embed a link. A placeholder shows up asking if the user wants to embed or convert it to a text link.

Use case.
User pasts a long list of links. Most of them show up as regular text links. A few show up with a placeholder asking if the user wants to embed or convert to text link.

@talldan talldan added [Block] Embed Affects the Embed Block [Type] Bug An existing feature does not function as intended labels Apr 7, 2020
@joaquimds
Copy link

joaquimds commented Apr 7, 2020

I would like Gutenberg to fallback to creating a regular hyperlink if the embed block is disabled

This is a reasonable fallback. Just to be sure, how are you disabling the Embed block?

@mcsf I'm using the allowed_block_types filter and not including the Embed block in the list of allowed blocks. Thanks for the movement on this issue!

@paaljoachim
Copy link
Contributor Author

So the process should be:
Link -> Can it embed? -> If yes -> Add a placeholder asking if the user wants to embed or convert to text link.
Link -> Can it embed? -> If no -> Automatically add a text link.

@desaiuditd
Copy link
Member

So the process should be:
Link -> Can it embed? -> If yes -> Add a placeholder asking if the user wants to embed or convert to text link.
Link -> Can it embed? -> If no -> Automatically add a text link.

@paaljoachim @mcsf Looking at the code in Embed block, seems like, the logic to check if the url can be embedded or not, tightly coupled into the edit component itself. So editor would have to render the embed block first, check for validity and then go back to paragraph block to show the text link. Not sure if this is good flow.

On the other hand, if we want to check for the validity inside the raw transform (that's what is executed as paste handler when user pastes a url into paragraph block), then I imagine, I would have to refactor existing embed block modules. Do we have consent of that big of a change? I wanted to get some thoughts on that, get a ✅ before I start implemnting it.

@paaljoachim
Copy link
Contributor Author

Hey @desaiuditd

Could you break down into more details what you said above?
I know I asked you about this in a direct message but it would also be good to have more details here as well. Thanks.

@paaljoachim
Copy link
Contributor Author

I made an Embed prototype. Check my post here:
#21789 (comment)

@ockham
Copy link
Contributor

ockham commented Aug 18, 2020

I would like Gutenberg to fallback to creating a regular hyperlink if the embed block is disabled

This is a reasonable fallback. Just to be sure, how are you disabling the Embed block?

@mcsf I'm using the allowed_block_types filter and not including the Embed block in the list of allowed blocks. Thanks for the movement on this issue!

I'm addressing this in #24559 (I think 😅 ). Specifically, that PR prevents the block from attempting to transform into a specialized embed block variation if that variation isn't registered.

@bobbingwide
Copy link
Contributor

3. One thing that we really should do, IMO, is make sure the WP embeds in the editor look like the final result in the front end. This seems to be specific to WordPress-type embeds. I haven't looked under the hood, so I don't know if it's just a style issue or if there's more to it. To me that's the only proper bug in this scenario. :)

I've investigated this problem in bobbingwide/oik#158

If the requirement is for all WordPress embeds to be shown in the editor then
some extra logic will be needed to prevent the unwanted style= attribute being added to the iframe

There's a related issue which questions if the blockquote should be displayed in the editor. #21945

In the frontend the blockquote is hidden. style="display: none".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants