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

<p> tags don't respect text-on-X-token #825

Closed
AgarwalPragy opened this issue Jan 13, 2023 · 9 comments
Closed

<p> tags don't respect text-on-X-token #825

AgarwalPragy opened this issue Jan 13, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@AgarwalPragy
Copy link
Contributor

AgarwalPragy commented Jan 13, 2023

Current Behavior

Seems like the color of text inside <p> tags is always black or white depending on the dark-mode. It doesn't respect the text-on-X-token

Steps To Reproduce

<p class="bg-primary-500 text-on-primary-token">The quick brown fox jumps over the lazy dog.</p>
<div class="bg-primary-500 text-on-primary-token">The quick brown fox jumps over the lazy dog.</div>

image

Anything else?

This is an issue especially when the container has the text-on-primary-token set, and there is a <p> tag nested inside the container.

@AgarwalPragy AgarwalPragy added the bug Something isn't working label Jan 13, 2023
@endigo9740
Copy link
Contributor

endigo9740 commented Jan 13, 2023

Some classes require ! to take precedence. In this case you would use:

<p class="bg-primary-500 !text-on-primary-token">Text</p>

Optionally, paragraph elements support the .unstyled class, which would excludes it from the typography.css styling. So you could do something like this if you do not wish to set !important:

<p class="unstyled bg-primary-500 text-on-primary-token">Text</p>

We've observed this behavior before, but the source is Tailwind itself. Class inheritance does not match vanilla CSS style inheritance, which can be very confusing the first time you experience it.

@AgarwalPragy
Copy link
Contributor Author

AgarwalPragy commented Jan 13, 2023

@endigo9740 As I mentioned, this issue is especially troublesome when the container has the text-on-primary-token applied.

<div class="bg-primary-500 !text-on-primary-token">
    <div>This respects the containers text-color</div>
    <h4>So does this</h4>
    <p>But this does not, despite the !</p>
</div>

The container is being styled. The <p> tag is coming from a child component. This, along with svelte's scoped styling behavior makes it so that it is impossible to color the text inside <p> tags without passing a class prop to every single component across the entire tree.

It seems to be due to .dark p:not(.unstyled)

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 13, 2023

In that case you would need to use:

<div class="bg-primary-500 !text-on-primary-token">
    <div>This respects the containers text-color</div>
    <h4>So does this</h4>
    <!-- NOTE THE CHANGE HERE --->
    <p class="unstyled">But this does not, despite the !</p>
</div>

I agree it's not an ideal solution, but we're juggling a lot of different styles here, including the need to exclude styles on demand. So currently this is the best way to go about it that I'm aware of.

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 13, 2023

One idea might be to exclude the paragraph styles altogether and allow them to inherit from the overall page text colors. I'm going to note this as something to test here:

@AgarwalPragy
Copy link
Contributor Author

AgarwalPragy commented Jan 13, 2023

In my case, the content is coming from a CMS, so I can't really control it.

This issue isn't due to tailwind as far as I can see https://play.tailwindcss.com/67W7GSQi7a
It seems to stem from the custom unstyled and .dark p:not(.unstyled)

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 13, 2023

My only concern with the above proposal would be unintended consequences - as in retroactively for existing users. Folks relying on those to have a hard set color, but then suddenly not. But overall it might be a better experience. It'll definitely require some degree of testing, so I would ask for your patience until we can both get to that ticket, and test this specifically.

Thanks!

@AgarwalPragy
Copy link
Contributor Author

AgarwalPragy commented Jan 13, 2023

I understand. No hurry - Skeleton is pre-1.0 so changes are a given. Plus I can't complain about FOSS :)

@AgarwalPragy
Copy link
Contributor Author

It seems that code blocks have the same issue? Skeleton seems to be forcing a style on codeblocks which renders all text in primary color for some reason.
I'm trying to use highlight.js to render content coming from a CMS

@endigo9740
Copy link
Contributor

endigo9740 commented Feb 1, 2023

@AgarwalPragy I'd recommend creating a new ticket for a new issue. Otherwise it's easy for stuff to get lost.

It depends how you're generating the page and how the content is coming into whether or not you can use a Svelte component like that. For example, on our blog, we don't use our Code Block component, but rather native pre tags and utilize Highlight.js directly after mount:

https:/skeletonlabs/skeleton/blob/dev/src/routes/blog/%5Bslug%5D/%2Bpage.svelte#L19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants