Closed schibrikov closed 1 year ago
vercel[bot]
commented
1 year ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated |
|---|---|---|---|---|
| svelte-select | ✅ Ready (Inspect) | Visit Preview | 💬 Add your feedback | Feb 7, 2023 at 1:24PM (UTC) |
schibrikov
commented
1 year ago Also I found out that "focused" doesn't always mean "opened". And in my case, since I'm setting border-radius for the "focused" state for select to play well with the list, it seems to be incorrect.
I'm wondering if you can propose any solution for it? Can we introduce "--border-radius-opened" property? If it's okay, I could also implement that.
rob-balfre
commented
1 year ago @schibrikov You are very right! Whooooops
Fixed it up and added a fallback to --border-radius-focused and an example to the site for style props.
Does this example help with your listOpen and radius problem?
https://svelte-select-examples.vercel.app/examples/advanced/style-props
Change in 5.2.1
schibrikov
commented
1 year ago @rob-balfre uhm, yeah, I think it is! It feels a bit "javascriptish", but I can live with that.
But what thinking about now is that I could also solve the original issue this way, since focused is exposed too. So I'm wondering now where is the line here - between introducing a separate customisation variable and offering a JS approach. Do you have any rule on that?
rob-balfre
commented
1 year ago @schibrikov hmm how about using :global or an external stylesheet and target .svelte-select.list-open then you could update the style props without having to bind:listOpen.
https://svelte.dev/repl/08fce4edd3a644ec98f72769b110ff57?version=3.55.1
schibrikov
commented
1 year ago @rob-balfre yeah, thanks, it feels better. A drawback here is we are leaking the internal component structure that is not a part of public API (unlike JS solution, which only uses documented props).
Well, anyway, my problem is solvable with these 2 options. I know introducing hundreds of css vars is probably not a way to go. So having these workarounds in mind there is no need for the further extension of the public API.
It seems that our yesterday changes have broken the original --border-radius property, because it's now overridden on the top-level despite being set. So it's incorrect to set the default value like this and we should only use fallback parameter in var(..) function.