robsontenorio / mary

Laravel Blade UI Components for Livewire 3
https://mary-ui.com
Other
1.06k stars 135 forks source link

[RFC] Default style for all inputs #585

Closed anfeichtinger closed 2 months ago

anfeichtinger commented 3 months ago

I have noticed that in some places classes like text-gray-500 or text-red-500 are used. To be fully compatible with (custom) daisy ui themes this should be avoided in my opinion.

I wanted to start a discussion if this is a welcome change. If it is I can create a pull request for it. Otherwise I have two approaches that could make up for it:

Otherwise users need to globally overwrite those usages with an css selector which is not optimal as it leads to some annoying custom css styles

Please note that this could be a breaking change for some projects.

robsontenorio commented 3 months ago

I would like to hear people.

But , you are right. It is better to have compliant css class from daisy instead of “raw classes”

flatcapco commented 2 months ago

Yep I agree this would be a great improvement. Also for example I pretty much have a class="border-gray-400" on all inputs as the default using the primary color feels quite over opinionated. so if classes are changing I'd vote for that to be a more neutral default. The primary works very well when selected but its overpowering when there are a lot of form fields.

Screenshot to show what I'd consider a better default: https://capture.dropbox.com/c6ucIhEdRMYzl94I

anfeichtinger commented 2 months ago

@flatcapco Thanks for your input on this issue.

Regarding the input style this has been discussed in #541 already. I'm not sure if that will be implemented as a default, though I would also be in favour of adding a config flag for this.

beholdr commented 2 months ago

Yeah, it would be great to have better support for a custom themes and ability to easy override styles. And I fully agree about more neutral colors for inputs by default. Primary colors are great to emphasize some elements, but currently it’s not so easy to do because they are hardcoded.

And for custom colors maybe it’s better to create a custom tailwind colors, like tokens in some design systems. E.g. mary-text-primary, mary-text-secondary, mary-bg-primary... And allow to override them with CSS vars.

robsontenorio commented 2 months ago

Let's try again about the default style for all inputs.

When you guys are building forms, 99% of time you don't want to use the primary style?

image

<x-input label="Name" wire:model="name" class="input-neutral" />
.input-neutral {
    @apply border-gray-400 focus:border-gray-400 focus:outline-gray-400
}

image

flatcapco commented 2 months ago

99% of the time I would want to use a more neutral style. I dont ever even see a reason I'd use the primary colour unless I really wanted to draw attention to a specific field which I've never actually come across yet in many systems.

The active style with primary is great. As it draws attention to where you are inputting.

beholdr commented 2 months ago

Same here. Almost always I want a default neutral style. Same as default input-bordered of daisy ui:

SCR-20240828-ornf

And in rare cases I want to highlight some inputs with input-primary class.

To always add an override classes is cumbersome. And overriding them via @apply in app.css as suggested in #541 overrides colors for error states too.

flatcapco commented 2 months ago
<x-input label="Name" wire:model="name" class="input-neutral" />
.input-neutral {
    @apply border-gray-400 focus:border-gray-400 focus:outline-gray-400
}

I get that but it seems like an anti pattern, which also becomes more of a pain on things like:

<x-input label="Default money" wire:model="money1" prefix="USD" money inline />
robsontenorio commented 2 months ago

I am running out of options 😢

Because the primary style for any input is the default and it would be huge breaking change I don't want to deal.

flatcapco commented 2 months ago

In this instance, my vote is that I think its actually worth the breaking change, and I do realise the fallout that comes with that.

beholdr commented 2 months ago

Breaking changes could could be mitigated using a config option, like 'primary_theme' => true.

anfeichtinger commented 2 months ago

I would also be in favour of making this breaking change (and adding the config option if necessary). It will not break functionality, only change the style a bit and from what I've seen most websites use neutral inputs anyways. When someone is updating their project with the new version it will be quick to also update their input styles if the don't want that.

thiagoradice commented 2 months ago

I've come across this exactly problem yesterday. Here is how I solved it:

input[type=text], input[type=password], input[type=number], input[type=email], textarea{
    @apply input-secondary;
}

Just add the input types selectors you want to change. I've gone with input-secondary styles, but you may change it for what suits you better.

anfeichtinger commented 2 months ago

I've come across this exactly problem yesterday. Here is how I solved it:

This seems like a reasonable workaround. One issue, textarea alone doesn't work for me, I needed to use textarea.textarea.\ If this should be the official workaround as it would be a breaking change then it should be put into the docs.

What I get is that from this discussion most people would like to use the following:

input[type=text], input[type=password], input[type=number], input[type=email], input[type=search], textarea.textarea {
    @apply input-bordered;
}

This results in the following input styles (focused + default)

image

@robsontenorio what do you think?

flatcapco commented 2 months ago

Sadly thats not the solution because using apply like that doesn't handle a full scenario. For example currency with suffix and prefix:

https://capture.dropbox.com/QqgcH4l54wclmIso

                <x-input suffix="R$" prefix="USD" inline money inline/>

Also your example with input-bordered brings another good point about a default being user selectable because I feel like the contrast is still too low using input-bordered and I usually settle on a gray-400

https://capture.dropbox.com/sDY66QrplPXdx6pz

robsontenorio commented 2 months ago

I think this will be addressed with maryUI v2 (upcoming daisyUI 5 + Tailwind 4), where I will introduce that breaking change. In a couple of days I will start the v2 branch and see how it goes.

I see that was a mistake to make all the input fields as primary.

robsontenorio commented 2 months ago

Please, follow

https://github.com/robsontenorio/mary/pull/623/files