SlRvb / Obsidian--ITS-Theme

Theme designed with readability and customizability in mind. Change it easily to your liking with the Style Settings plugin installed.
https://publish.obsidian.md/slrvb-docs/ITS+Theme/ITS+Theme
GNU General Public License v2.0
521 stars 113 forks source link

[Bug] Size of embedded image and container don't match #207

Closed Jnosh closed 9 months ago

Jnosh commented 1 year ago

Describe the bug

When embedding an image with a width specifier, the img element width is set in pixels but the span container width is set percentage relative to the page width. This leads to a visual inconsistency where the container grows/shrinks with the window size but the image does not...

E.g. for

![[image.jpg|left wmed]]

the span container gets the following style:

<style>
.internal-embed[alt*=wmed] {
    width: 60%;
}

whereas the img element uses the following:

img[alt][alt][alt*=wmed], .image-embed[src*="#wmed"] img, .image-embed[src*="#wmed"]::after {
    width: var(--medium);
}

According to the documentation, the image sizes should always be in pixels although I would love to also see an option to use relative image sizes for plain image embeds. Callouts already allow using static or dynamic sizes using static so I would love to see the same ability for plain image embeds as well.

OS

Where does the bug occur?

Versions

SlRvb commented 1 year ago

If you update to ITS v1.0.32 and add relative in the image alt text, does that do what you want it to?

Jnosh commented 1 year ago

relative seems to work to get relative sizes for images, thanks.

However, with both static and dynamic sizes the image container still seems to be sized differently from the actual image:

Screenshot 2023-06-04 at 23 44 40 Screenshot 2023-06-04 at 23 45 35

The container is always using a relative size and is not using the CSS variables that the image size is using. So with an e.g. wide page, there is empty space between the floating image and the regular content since the container is wider than the image.

Jnosh commented 1 year ago

For reference, the markdown used for the above examples:

![[image.png|left wsmall]]

Sed ad incididunt qui incididunt amet duis enim mollit in duis lorem exercitation sunt nisi elit dolore. Minim sit commodo esse esse velit ullamco ex duis nulla magna consequat adipiscing mollit. Id eu deserunt incididunt occaecat eiusmod est laboris adipiscing et velit velit anim pariatur sed non elit labore elit. Sunt labore id exercitation ex nisi laboris ex reprehenderit aliquip nisi nisi fugiat aliqua do ea mollit aliquip nostrud.

I think for the default (static) case, these widths are applied to the image container.

The image in that case has a fixed width in pixels and the container a relative width in percent. So the container grows and shrinks along with the page width but the image can only shrink since it has a fixed max width but shrinks to match the container size.

The image container should probably not grow bigger than the image size so there isn't a big gap between the floating image and content for wider page/window width. i.e. they should both have the same size or at least the same max size.

The shrinking doesn't look bad and is convenient to keep the image floating and never overflow the page width. But in theory also contradicts the "fixed image size" attributes. I can see arguments either way - depends on what your intention is for the image size attributes in this case.


For the new relative width case, the issue seems to be that the relative width is applied twice:

![[image.png|left wsmall relative]]

Sed ad incididunt qui incididunt amet duis enim mollit in duis lorem exercitation sunt nisi elit dolore. Minim sit commodo esse esse velit ullamco ex duis nulla magna consequat adipiscing mollit. Id eu deserunt incididunt occaecat eiusmod est laboris adipiscing et velit velit anim pariatur sed non elit labore elit. Sunt labore id exercitation ex nisi laboris ex reprehenderit aliquip nisi nisi fugiat aliqua do ea mollit aliquip nostrud.
Screenshot 2023-06-05 at 00 09 22

In that case, both the image container and the image have a relative width applied, so the image container is relative to the page width and the image is relative to the image container width:

// Image container - sets a width relative to the whole page
.image-embed[alt][alt*=relative][alt*=wsmall], div:not(.image-embed) > img[alt][alt*=relative][alt*=wsmall] {
    width: var(--small);
}

// Image inside the container - also has a relative width - but this is now relative to the container.
img[alt][alt][alt*=wsmall] {
    width: var(--small);
}

Probably just the image container should have a relative size and the image itself should just fill the container width in this case or something like that.

SlRvb commented 1 year ago

Any chance this might be fixed in v1.0.47+? Otherwise I'll try to recreate it again

Jnosh commented 1 year ago

I can still reproduce it in my test vault.

I've attached the note I use to test for this issue here. The image I used is just the ITS logo from the documentation website but any image should work.

Test Note.md

SlRvb commented 1 year ago

I think this should now be fixed in ITS v1.0.50 :)

Jnosh commented 1 year ago

It does appear to work as expected when using relative, but not yet without relative.

The span into which the image is embedded seems to always use a relative size.

CSS applied to img element (using left wsmall):

Screenshot 2023-06-26 at 13 12 03

CSS applied to the span containing the img:

Screenshot 2023-06-26 at 13 12 22
SlRvb commented 9 months ago

Okay I think this latest update v1.1.24 should actually fix this issue now. Sorry it took so long, I was having a hard time finding the source of the problems, but it should all be fixed now 🙏🏽

Jnosh commented 9 months ago

I can confirm that it works as expected now! 🎉

Thanks for all the effort!