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 literal string as class in view macro #500

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

g-re-g
Copy link
Contributor

@g-re-g g-re-g commented Feb 11, 2023

Render literal string as class in view macro correctly. This is currently allowed but produces incorrect html.

this code:

// Notice that "scope-class" doesn't have braces around it `{ ... }`
view!{cx, class="scope-class", ...}

Pre this PR:
Screenshot 2023-02-10 at 7 51 12 PM

Notice there's an extra attribute scope-class""="".
This is because the quotes in "scope-class" get rendered and the browser turns that into its own attribute.

Post this PR:
Screenshot 2023-02-10 at 7 50 00 PM

The class renders correctly.

As a side note, the Header and Banner component that are inside of the scope-class scope don't have the scope-class applied. I don't know if this is intentional because they are components, but there it is.

This PR also includes updated error messaging if a user tries to use a class but forgets the trailing comma.

Pre this PR it would say:

error: expected literal                                                                                                                                           
   --> src/app.rs:299:17                                                                                                                                          
    |                                                                                                                                                             
299 |     view! { cx, class="scope-class"                                                                                                                         
    |                 ^^^^^        

Now it says:

   --> src/app.rs:299:5
    |
299 | /     view! { cx, class="scope-class"
300 | |       <div class="home-page">
301 | |         // Header
302 | |         <Header />
...   |
340 | |       <Footer/>
341 | |     }
    | |_____^
    |
    = help: message: To create a scope class with the view! macro you must put a comma `,` after the value.
            e.g., view!{cx, class="my-class", <div>...</div>}

Comment on lines -151 to +178
if nodes.is_empty() {
let span = Span::call_site();
quote_spanned! {
span => leptos::Unit
match nodes.len() {
0 => {
let span = Span::call_site();
quote_spanned! {
span => leptos::Unit
}
}
} else if nodes.len() == 1 {
root_node_to_tokens_ssr(cx, &nodes[0], global_class)
} else {
fragment_to_tokens_ssr(cx, Span::call_site(), nodes, global_class)
1 => root_node_to_tokens_ssr(cx, &nodes[0], global_class),
_ => fragment_to_tokens_ssr(cx, Span::call_site(), nodes, global_class),
}
} else if nodes.is_empty() {
let span = Span::call_site();
quote_spanned! {
span => leptos::Unit
}
} else if nodes.len() == 1 {
node_to_tokens(cx, &nodes[0], TagType::Unknown, global_class)
} else {
fragment_to_tokens(
cx,
Span::call_site(),
nodes,
true,
TagType::Unknown,
global_class,
)
match nodes.len() {
0 => {
let span = Span::call_site();
quote_spanned! {
span => leptos::Unit
}
}
1 => node_to_tokens(cx, &nodes[0], TagType::Unknown, global_class),
_ => fragment_to_tokens(
cx,
Span::call_site(),
nodes,
true,
TagType::Unknown,
global_class,
),
}
Copy link
Contributor Author

@g-re-g g-re-g Feb 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a rearrangement, no functional changes. While reading it this is how I rearranged the logic in my head and figured I'd update the code to be the same. Happy to revert if the old way is preferred.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@gbj
Copy link
Collaborator

gbj commented Feb 11, 2023

Thanks, this looks great and I appreciate the clean-up work here too.

@gbj gbj merged commit d0cacec into leptos-rs:main Feb 11, 2023
gbj pushed a commit that referenced this pull request Feb 12, 2023
gbj pushed a commit that referenced this pull request Mar 21, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants