-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat: customizable select #17429
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
base: main
Are you sure you want to change the base?
feat: customizable select #17429
Conversation
🦋 Changeset detectedLatest commit: 89ed7df The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
Any thoughts on how we could handle a case like this? |
|
Uhhhh that's true...I wonder if we should disallow components all together it's a bit limiting but we do need the text value at compile time which is simply not possible if component are involved no? Maybe we could have a separate state in the walk and we only push expressions and text to it? |
|
Do we need it at compile time? Could it be as simple as just bailing out of hydration inside |
|
Ok this is hilarious because that's actually the solution Claude came up with initially but I thought it was bad...I guess that could work tho...we disable hydration, we run the function and then re-enable hydration. It wouldn't even need to have two function because it would work regardless no? I just wonder if this could mess up hydration in some way but it shouldn't 🤔 |
|
Yeah, it should work just fine |
Mmm it's not that simple...it could work if the child of an option is only a component but if we have something like this <option><span>{something}</span> <Component /></option>the code tries to get the first child of option and the text inside it anyway and since the |
|
Can we do the opposite: hydrate and fix options (back) into rich options? It seems you can insert whatever content, but in browsers without customizable select support, only |
WDYM the opposite? The point is that we should hydrate in both cases, in one case by actually picking the |
|
I mean, fix it to only the rich variant (in the example, insert |
|
opened #17436 |
|
One more TODO is, in CSS selectors, treat |
Closes #15617
This is not the whole PR but is technically mergeable as is: what this does is that if there's an option with rich content inside instead of producing the code we produce today it produces code that looks like this
rich_optionis a function that checks if the new parser that persists elements insideoptionis in use (unfortunately we need to use this trick of creating an element, setting it and checking the first element because Safari preview ships with the new parser butCSS.supportsstill doesn't support stylable selects, so the new parser could be in place without the CSS support being there ref) and invokes one of the two branches. The two branches are either "only text" in case of the old parser or the whole tree in case of the new parser.What's missing for full support:
buttoninside selectselectedcontentinsidebuttonbuttonhas state inside because the old parser will just remove it completelyoptgroup(I need to check what's the actual behaviour)I tested it in browsers that supports the new parser and browsers that don't, and it seems to work.
Warning
Claude Opus 4.5 actually wrote most of the code...I reviewed it and corrected it where I thought it was wrong but please use extra caution in reviewing this PR 🧡
As I've said, this is technically mergeable, and it will allow initial stylization of the select (if you don't use
buttonwithselectedcontent)...I plan to fully implement everything but wanted an initial feedback on this.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint