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

Fixes to the way Render handles attributes #45

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

theashguy
Copy link
Collaborator

@theashguy theashguy commented May 27, 2021

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.

@theashguy theashguy changed the title Fixes to the way Render handles tags Fixes to the way Render handles attributes May 27, 2021
escape_html(&value, writer)?;
write!(writer, "{}", value)?;
Copy link
Collaborator Author

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);
}
Copy link
Collaborator Author

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![=]);
}
Copy link
Collaborator Author

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(),
}
)
Copy link
Collaborator Author

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' }"/>"#
);
}
Copy link
Collaborator Author

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.

@theashguy
Copy link
Collaborator Author

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();
Copy link
Collaborator Author

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('.') == '!' {
Copy link
Collaborator Author

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.

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.

1 participant