delucis / astro-embed

Low-JavaScript embed components for Astro websites
https://astro-embed.netlify.app
MIT License
172 stars 25 forks source link

Blank `<p>` added between "Tweet" embeds #65

Closed mattrossman closed 9 months ago

mattrossman commented 9 months ago

When multiple Twitter links appear in an MDX file, there is a blank <p> tag between them, which can create awkward gaps on the page when markdown is styled.

Minimal Reproduction:

https://codesandbox.io/p/sandbox/astro-embed-tweets-extra-p-issue-4z4zq3?file=%2Fsrc%2Fpages%2Findex.mdx%3A1%2C1

Notice how there is a large gap between the two tweets, but the gap is not present between YouTube embeds.

2x Tweets markup (each item is prefaced by a blank <p><astro-embed-tweet></p> and followed by a blank <p></p>:

CleanShot 2023-10-20 at 17 34 10@2x

2x YouTube markup (each item is in a single <p>:

CleanShot 2023-10-20 at 17 32 44@2x

I would like the Tweet embed to behave similar to the YouTube one, so it just lives under a single element for consistent spacing.

mattrossman commented 9 months ago

Narrowed down the issue to the usage of set:html in Tweet.astro. It produces a single element without this directive, but once the directive is added it bleeds other elements outside of the root <p>

It seems to be sensitive to the kinds of elements present in the inner HTML.

For instance this (using <span>):

<Fragment set:html={`<astro-embed-tweet><span>test</span></astro-embed-tweet`} />

Produces the markup you'd expect:

<p>
  <astro-embed-tweet>
    <span>test</span>
  </astro-embed-tweet>
</p>

But this (using <div>):

<Fragment set:html={`<astro-embed-tweet><div>test</div></astro-embed-tweet`} />

Produces this with the <div> in a different spot in the markup.

<p>
  <astro-embed-tweet></astro-embed-tweet>
</p>
<div>test</div>
delucis commented 9 months ago

Thanks for the report and the investigation! This is actually an issue with HTML parsing. If you look at the HTML generated, it looks like this:

<p>
  <astro-embed-tweet>
    <blockquote class="twitter-tweet" data-dnt="true">
      <p lang="en" dir="ltr">Tweet</p>
      &mdash; Username (@handle) <a href="...">Month D, YYYY</a>
    </blockquote>
  </astro-embed-tweet>
</p>

The problem is that <blockquote> is not permitted inside <p>, so when the browser parses this, it does something like:

  1. <p> — OK, starting a paragraph

  2. <astro-embed-tweet> — custom element, adding it inside our paragraph

  3. <blockquote> — uh oh, they forgot to close the <p>, no problem, we can do that for them (and close any children too) and then keep going.

So the result is:

<p>
  <astro-embed-tweet>
  </astro-embed-tweet>
</p>
<blockquote class="twitter-tweet" data-dnt="true">
  <p lang="en" dir="ltr">Tweet</p>
  &mdash; Username (@handle) <a href="...">Month D, YYYY</a>
</blockquote>

Whether or not you get the large gaps, depends on your CSS (in your reproduction display: grid on the <section> containing the embeds), so I missed this in the test site. This problem impacts https://astro-embed.netlify.app/integration/ for example, but there’s no custom CSS so there’s no visual impact.

I’ll note this is only an issue with the embeds() MDX integration, if you use the component directly, there’s no extra <p>:

import { Tweet } from 'astro-embed'

<Tweet id="https://twitter.com/astrodotbuild/status/1511750228428435457" />

We can probably fix the embeds() remark plugin to remove the parent <p> tag in these cases to fix this.