Skip to content
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

Allow multiple classes in the class-tuple syntax #2522

Closed
bicarlsen opened this issue Apr 13, 2024 · 4 comments
Closed

Allow multiple classes in the class-tuple syntax #2522

bicarlsen opened this issue Apr 13, 2024 · 4 comments

Comments

@bicarlsen
Copy link
Contributor

bicarlsen commented Apr 13, 2024

Is your feature request related to a problem? Please describe.
When using the class tuple syntax (i.e. class=("my-class", true)), multiple classes are not supported. e.g. class=("class-1 class-2", true).

Describe the solution you'd like
Extend the class-tuple syntax to allow for multiple classes.

Describe alternatives you've considered
I currently move each of the classes into its own tuple. e.g. class=("class-1", true) class=("class-2", true).

Additional context
The issue seems to come because DomTokenList.add_1(class_name) is used to add the class, which expects the classes to already be separated.

This could be remedied by first parsing the classes into a Vec, splitting on whitespace, then

  1. Using DomTokenList.add() in place of add_1, or
  2. By iterating over the Vec, repeatedly calling add_1
    I haven't played with the performance tradeoffs.
@bicarlsen
Copy link
Contributor Author

bicarlsen commented Apr 13, 2024

If this seems like something worth tackling, I'm happy to issue a PR.

@gbj
Copy link
Collaborator

gbj commented Apr 13, 2024

I am open to providing some alternate API to simultaneously set or un-set multiple classes in response to the same reactive change, and possibly checking for whitespace in the class name in the view macro and splitting them at runtime.

I'm not open to splitting the string at runtime for class:/class=(_, _) in general unless it's purely opt-in: if class:foo=some_condition requires checking whether "foo" contains whitespace or wrapping it in a JS Array, it's just pure overhead.

@bicarlsen
Copy link
Contributor Author

I'll take a look and see what I can put together. Agreed that it should be done in the view macro.

@gbj
Copy link
Collaborator

gbj commented Apr 17, 2024

Feature added by #2532. Nice work!

@gbj gbj closed this as completed Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants