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

Tag with class attribute with multiple classes fails: <div class="a b"> . #286

Closed
dilvan1 opened this issue Jun 18, 2018 · 5 comments
Closed

Comments

@dilvan1
Copy link

dilvan1 commented Jun 18, 2018

Description

I'm submitting a ... bug report

When a class attribute has more than 1 class, the program reports an error.

Expected Results

<div class="a b"> should work

Bug fix

File src/virtual_dom/vtag.rs:

80    pub fn add_classes(&mut self, classes: &str) {
81        let classes = classes.trim().split(" ");
82        for class in classes {
83            let class = class.trim();
84            if !class.is_empty() {
85                self.classes.insert(class.into());
86            }
87        }
88    }
@limira
Copy link
Contributor

limira commented Jun 18, 2018

Source code: It expect you input something like: <div class=("a", "b",), >, you can try it without trailing comma to see if it works?? (such as: <div class=("a", "b"), > I don't know 😁 ).

Or maybe we should accept class="a b c", then split them by space and add them, @deniskolodin ? Sorry for the hasty reply (I am not in good mood now, 😁 ), It is the proposal for fixing this by the OP himself, I just came up with the same idea!.

@therustmonk
Copy link
Member

Actually I hate this thing too 😢 I think I have to split string of classes in templates in the nearest PR.
Related #121

@limira
Copy link
Contributor

limira commented Jun 19, 2018

  1. class="a b c" then split before add: impact on performance as you mention in Can't use spaces directly in a class string #121. But if I remember correctly, it is the prefer way in HTML.
  2. class=("a", "b",),: You hate it yourself!
  3. In Experiment: shtml! #285 , I think I successfully (it pass the test, and I see the result in browser inspector) implement this syntax: class="a" "b" "c",. This way, it do not impact on performance, less awkward than (2.)

Maybe the solution is:

  • Allow both: class="a b c", (with a warn about performance in README?) and class="a" "b" "c",
    then users may choose what they want

@dilvan1
Copy link
Author

dilvan1 commented Jun 19, 2018

I didn't see the related open bug, sorry. The fix I've sent you, doesn't seem to impact compiling time.

@therustmonk
Copy link
Member

Fixed with #289

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants