Skip to content
This repository has been archived by the owner on Oct 16, 2021. It is now read-only.

Transform adds figure to paragraph #122

Open
thomasmassmann opened this issue May 11, 2020 · 5 comments
Open

Transform adds figure to paragraph #122

thomasmassmann opened this issue May 11, 2020 · 5 comments
Labels
discussion This issue is being fleshed out in the thread

Comments

@thomasmassmann
Copy link
Contributor

The transformation of article images to figures replaces the image with a figure and figcaption. Now figures are not allowed in p tags, which by default is the parent of the image. Some browsers will render empty p tags above and below the figure, which leads to additional unwanted space.

I fixed this locally by changing the transformation to this:

image.parentElement.replaceWith(figure)

Is the assumption ok that every image (from markdown content) will have a surrounding p tag? Or would it be better to test for the parent and its content?

@equivalentideas
Copy link
Contributor

I've had this issue as well. Thanks for investigating @thomasmassmann 💐

@thomasmassmann
Copy link
Contributor Author

I'm getting closer to what I want. If you have images with text before/after it, the text in my first patch is gone. Now the image is removed from the parent (p tag) and then added right after the parent if there is still some text in it.

figure.appendChild(image.cloneNode(true));
figure.appendChild(figCaption);
const parent = image.parentElement;
parent.removeChild(image);
if (parent.innerHTML.length) {
    parent.parentNode.insertBefore(figure, parent.nextSibling);
} else {
    parent.replaceWith(figure);
}

Now when you have more than one inline image it would be great to split the parent and have the new figure sit in between. Any hints?

Origanal behavior

![The top of a grey concrete building with a blue sky in the background](/images/demo-image-1.jpg "Brutalism at its finest. Photo by Artificial Photography on Unsplash.")

becomes

<p>
  <figure>...</figure>
</p>

which browsers will render as

<p></p>
<figure>...</figure>
<p></p>

With current patch

Image only

![The top of a grey concrete building with a blue sky in the background](/images/demo-image-1.jpg "Brutalism at its finest. Photo by Artificial Photography on Unsplash.")

becomes

<figure>...</figure>

Image inline with text

Some text before.
![The top of a grey concrete building with a blue sky in the background](/images/demo-image-1.jpg "Brutalism at its finest. Photo by Artificial Photography on Unsplash.")
Some text after.

becomes

<p>Some text before. Some text after.</p>
<figure>...</figure>

Multiple image inline with text

Some text before.
![The top of a grey concrete building with a blue sky in the background](/images/demo-image-1.jpg "Brutalism at its finest. Photo by Artificial Photography on Unsplash.")
Some text after.
![The top of a grey concrete building with a blue sky in the background](/images/demo-image-1.jpg "Brutalism at its finest. Photo by Artificial Photography on Unsplash.")
End of text.

becomes

<p>Some text before. Some text after. End of text.</p>
<figure>...</figure>
<figure>...</figure>

But it should be

<p>Some text before.</p>
<figure>...</figure>
<p>Some text after.</p>
<figure>...</figure>
<p>End of text.</p>

@Andy-set-studio
Copy link
Owner

I appreciate the work going into this. I'm a bit stacked to be thinking about it right now, but it's certainly one to keep an eye on.

@Andy-set-studio Andy-set-studio added the discussion This issue is being fleshed out in the thread label May 12, 2020
@thomasmassmann
Copy link
Contributor Author

Sure, that's nothing super critical to worry about right now. I want to discuss things first and see what possible problems and side effects are possible before creating a PR. And also get some feedback from others who use hylia for their projects.

@Andy-set-studio
Copy link
Owner

Thank you, Thomas. I'm really glad you spotted this because I certainly don't want to be shipping bad HTML :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion This issue is being fleshed out in the thread
Projects
None yet
Development

No branches or pull requests

3 participants