search

noatpad / obsidian-banners

An Obsidian plugin that adds banners to your notes
MIT License
616 stars 39 forks source link

Incompatibility with Minimal Theme #130

Open Aeases opened 11 months ago

Aeases commented 11 months ago

Only happens with the Minimal theme for me, othersl ike anuppucin and the default theme work fine

image

noatpad commented 11 months ago

Gonna give this a looksee if this is something that can be resolved within Banners, as well as if it's something to be fixed from the alternative banner placement that I mentioned earlier (#129)

RyotaUshio commented 11 months ago

It seems that this part of Minimal's theme.css is the problem:

.minimal-line-nums .workspace-leaf-content[data-type=markdown] .is-readable-line-width {
    --file-margins: 1rem 0 0 var(--folding-offset)
}

and this --file-margins variable is referenced by this part of Banners' style.css, which styles the grandparent div element of the banner img:

.obsidian-banner-wrapper.with-banner:not(.in-internal-embed) {
    width: calc(100% + 2 * var(--file-margins));
    margin: calc(-1 * var(--file-margins));
    margin-bottom: var(--file-margins);
}

--file-margins is var(--size-4-8) = 32px in the default theme, so width: calc(100% + 2 * var(--size-4-8));can be a quick fix. Of course this may not be a proper solution, but I'm not sure how this should be resolved cuz I don't know much about CSS...

noatpad commented 11 months ago

Mm, so it's sort of two-fold because of how this variable specifically (and probably others) are treated as single-valued and multi-valued. This one in Obsidian defaults, for example, is treated as var(--size-4-8) on desktop, but on mobile it's var(--size-4-2) var(--size-4-5). I think what I'll do is encapsulate more CSS variables within Banners and set their defaults to more atomic variables found in Obsidian (so I'll probably make a --banners-margin-horizontal and --banners-margin-vertical for these and assign them to the aforementioned variables). This'll give the right defaults and more control for themes.

Now with that said, I think there'll still be some theme incompatibility out-of-the-box, but I do think that will be up to the theme to keep that in check (though for now, I'd rather wait until Banners is out of beta to avoid outdated changes for themes, since I'm still moving things around)