skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
4.99k stars 317 forks source link

Invasive styles added to various components #908

Closed AgarwalPragy closed 1 year ago

AgarwalPragy commented 1 year ago

Current Behavior

This is what the following looks like with the default theme

<pre><code>for (let i = 1; i &lt;= 100; i++) {
    let out = '';
    if (i % 3 === 0) out += 'Fizz';
    if (i % 5 === 0) out += 'Buzz';
    console.log(out || i);
}</code></pre>
image

Notice how both the text and the background use primary color for some reason instead of inheriting from parent container.

Similar styles have been applied to h1-h6, pre, code, p, kbd, samp, and several other tags (I think).


If a wrapper container with class prose is added, then everything becomes unstyled and stops inherting parent styles of used theme - everything goes to hell.


Overall, the design system seems poorly thought out and needlessly invasive.

Rendering dynamic markdown content from a CMS is exceedingly difficult.

Steps To Reproduce

No response

Anything else?

No response

endigo9740 commented 1 year ago

Overall, the design system seems poorly thought out and needlessly invasive.

@AgarwalPragy Just a friendly heads up we enforce a code of conduct on both Discord and here on Github: https://github.com/skeletonlabs/community/blob/main/CODE_OF_CONDUCT.md

image

Criticism is welcome, but be mindful of stepping over the line into being rude or condescending. This project is created and maintained by normal human beings, volunteers at that, completely for free for you and others. If you're not happy with how Skeleton works or operates there's plenty of alternatives available to you.

AgarwalPragy commented 1 year ago

No disrespect was intended. I understand that this is free open-source software, and I've no reason to complain. I'm sorry if it came out as rude.

The statement about things being poorly thought out is a valid observation. The project doesn't follow the principle of sane defaults and several things need re-thinking from the ground-up.

endigo9740 commented 1 year ago

Intended or not, that's how it came across. And this is not the first occasion. Again, if you're not happy with the way things are setup and run, the fork button exists up there at the top of the repo. Click it and do as you please if you think you can do better.

This is your verbal warning. GitHub doesn't allow for timeouts, so next step is a ban. Thanks for your understanding.

riziles commented 1 year ago

@endigo9740 , kudos to you for your patient and considerate response.

I don't know much, but as far as I can tell, Skeleton implements Tailwind exactly as it's designed (that said, I learned Tailwind last week, so my opinion isn't that relevant). FWIW, I use mdsvex inside of Skeleton, and it works just fine, and that includes code syntax highlighting.

AgarwalPragy commented 1 year ago

Intended or not, that's how it came across.

Fair enough. I will keep that in mind. đź‘Ť

And this is not the first occasion

I'm genuinely surprised. Would you be kind enough to point out the instances?

Again, I didn't mean to be rude. I've nothing but respect for the open-source community.

AgarwalPragy commented 1 year ago

@riziles That is surprising. Were you able to reproduce the bug I mentioned? Could you share a screenshot of what the following code produces with your setup? Thanks.

<pre><code>for (let i = 1; i &lt;= 100; i++) {
    let out = '';
    if (i % 3 === 0) out += 'Fizz';
    if (i % 5 === 0) out += 'Buzz';
    console.log(out || i);
}</code></pre>
AgarwalPragy commented 1 year ago

@riziles

I don't know much, but as far as I can tell, Skeleton implements Tailwind exactly as it's designed

Tailwind is explicit about not using default element styles. It infact removes all default element styles! https://tailwindcss.com/docs/preflight

It is further explained in this post https://tailwindcss.com/blog/tailwindcss-typography

Why is Tailwind removing the default styles on my h1 elements? How do I disable this? What do you mean I lose all the other base styles too?

"We hear you, but we’re not convinced that simply disabling our base styles is what you really want. You don’t want to have to remove annoying margins every time you use a p element in a piece of your dashboard UI. And I doubt you really want your blog posts to use the user-agent styles either — you want them to look awesome, not awful."


Skeleton seems to be adding back styles to several elements, which ends up causing the exact problem that tailwind aims to solve with preflight - the user has to remove those styles every time they use an element in their UI.

riziles commented 1 year ago

Could you share a screenshot of what the following code produces with your setup? Thanks.

@AgarwalPragy , MDsveX is a Markdown processor, so I use the triple backtick syntax instead of <pre><code>. The output looks like this:

image

endigo9740 commented 1 year ago

Skeleton seems to be adding back styles to several elements, which ends up causing the exact problem that tailwind aims to solve with preflight - the user has to remove those styles every time they use an element in their UI.

Have you considered maybe asking WHY we did this? That might be a good place to start, rather than assuming ignorance.

Have you considered the fact that maybe we also share the opinion that global styles have some downsides? Perhaps even looked a couple issues down from your own at a proposal to change this for input styling? https://github.com/skeletonlabs/skeleton/issues/900

This is what I'm trying to communicate Agarwal. You're coming in here acting like everyone is below you and doesn't know what they're doing, but you're doing yourself no favors. Additionally nearly every one of your post seems to come across as a demand rather than a request.

Remember we don't work for you. If you're making demands of something being offered for free, that's really entitled stance and not a productive way to participate in open source. If you want to help, then help - alert us to your issues, then be patient and give us time to address said issue. But coming in and tearing down folks hard work is not going to fly here.

That said, if you wish to continue this conversation I might suggest doing so on Discord or a GitHub Discussion thread. This is way off topic and out of scope for this thread.

riziles commented 1 year ago

@endigo9740 , apologies for re-engaging and re-opening. Will take this to a better forum. Love your work, and thank you for everything you do.

endigo9740 commented 1 year ago

@riziles that was not a comment directed at you! My bad for not specifying!

endigo9740 commented 1 year ago

I've pushed out the following fix for pre code block styles: https://github.com/skeletonlabs/skeleton/commit/fea03a5199b28e61f7825193f9814069023aa098

Here's the result:

Screenshot 2023-02-03 at 1 51 06 PM

Note that this WON'T automatically enable highlight.js - you'll still need to follow their instructions for querying and applying syntax highlighting. But it will stripe the code styles from ever affecting the innards of a pre tag. Giving you a blank slate essentially.

This is already merged into dev branch, which means it will be part of next week's release.