-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fixes to the way Render handles attributes #45
base: master
Are you sure you want to change the base?
Conversation
render/src/simple_element.rs
Outdated
escape_html(&value, writer)?; | ||
write!(writer, "{}", value)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we stop escaping the html of data that goes into attributes. This does mean library users will need to do this themselves. In future I'd suggest some kind of marker that would escape this unless it was attached.
|
||
let punct = input.parse().unwrap(); | ||
name.push_punct(punct); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loop is similar to the implementation in parse_separated_nonempty_with
. Key thing I was looking to do here was to give us the ability to modify how and when this accrues / breaks so we can better handle the edge cases. I'm not sure I ended up going very far with this but think it probably makes sense to hold onto this here.
|
||
if !not_punned { | ||
not_punned = input.peek3(syn::Token![=]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there are multiple sets of punctuation that can occur in a row we look forward to see if any of the next 3 Punctuated
's have an equals in it. This may ultimately become a limitation of the library that it can only handle 2 distinct piece of punctuation before the equals though that would be a vast improvement.
Some(p) => p.as_char().to_string(), | ||
None => "".to_string(), | ||
} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This properly collapses our punctuation and ident fields back into a proper string when there are multiple.
html! { <input x-bind:class={"{ 'active': tab === 'foo' }"} /> }, | ||
r#"<input x-bind:class="{ 'active': tab === 'foo' }"/>"# | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block is the only addition to the tests-- the rest is rustfmt.
Have added my notes for review. @Schniz @vpzomtrrfrt @Tarnadas |
fn render(self) -> String | ||
where | ||
Self: Sized, | ||
{ | ||
let mut buf = String::new(); | ||
self.render_into(&mut buf).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stuff makes the Render trait more portable-- we still need sized as its a dependency of the writer structure we're doing, but we can at least scope this requirement to the render method so that it'll only complain about being passed around when used directly in the html! macro... 🤞
write!(writer, " {}=\"", key)?; | ||
escape_html(&value, writer)?; | ||
write!(writer, "\"")?; | ||
if key.chars().nth(0).unwrap_or('.') == 'b' && key.chars().nth(1).unwrap_or('.') == '!' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not my favorite addition as its a bit of a hack-- definitely want a better way to do this in the future. This interprets b! at the start of an attribute as a boolean attribute in HTML, which will cause it to optionally render the key based on a value of "true".
Mostly because we need some way to do this today as things like selected='false' in an option isn't actually valid html.
Prior to this PR Render could only handle attributes without any punctuation in them OR hyphenated attributes. There are however a range of weird and wonderful attributes inside SVG and that various Javascript libraries use that require other types of punctuation OR multiple instances of punctuation in an attribute.
This PR improves the state of this (though I believe there are still some holes in how it works-- eg, I'm not sure it works if attributes begin with punctuation).
Additionally, it opens up the ability for attribute content to have special characters in it-- a requirement if you want to pre-load some data into a JS script for instance as a json object.