svelteuidev / svelteui

SvelteUI Monorepo
https://svelteui.dev
MIT License
1.29k stars 64 forks source link

Cannot set class on TextInput input DOM element #296

Closed drewbitt closed 1 year ago

drewbitt commented 1 year ago

What package has an issue

@svelteuidev/core

A clear and concise description of what the bug is

A TextInput when rendered creates something like

<div>
  <div>
     <input type="text">
  </div>
</div>

When setting class on the TextInput, like you would do in Tailwind, you might expect it to apply the classes to the input element (or at least have the option to do so with a similar class prop). However, it sets the classes on the first div element two layers up.

Workaround (For now)

You could use some of the classes svelteui is putting on the input, either in an external CSS selector, via css parent-child chaining, or in a :global style tag e.g.

<style>
       // can also scope this with ref
    :global(.svelteui-Input-input) {
        background-color: transparent !important;
    }
</style>

However I haven't tested if this classname will always be there in production

In which browser(s) did the problem occur?

No response

Steps To Reproduce

Use svelteui 0.9

Do you know how to fix the issue

Yes

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Relevant Assets

No response

drewbitt commented 1 year ago

Note that if you fall back to Input, which is of course documented with advise not to use directly, the last class name will be the variant, overwriting all other class set values as it appears last in the list. This forces the use of the headless variant.

Additionally, using an InputWrapper is not 1:1 as things like setting error=true does not change input border color to red like when using a TextInput

BeeMargarida commented 1 year ago

Hi, sorry, this got forgotten. Thank you so much for this bug report, it is really detailed. We normally set the class prop to the outmost element in the components, but perhaps it is not the best approach. We will have to rethink it

drewbitt commented 1 year ago

I came across a similar scenario today with Modals; the styling of the Modal header, which the title text prop is in, cannot be set. I imagine other scenarios exist where inner elements have desired styling. Therefore, I am happy with setting styles on the classes themselves which I outlined ways of doing in my original post, e.g. .svelteui-Modal-header or .svelteui-Input-input. Rather than adding more props on all elements where they have inner elements - which of course is still a possible solution. However the avenue of direct class styling should be added to the documentation, at least a FAQ outlining where to find css classnames. Svelte seemed to keep the classnames when I ran the production buid with vite preview

BeeMargarida commented 1 year ago

Ended up reverting the change, if we want to change the style, it's better to use global stylings or the style API