Skip to content

Commit

Permalink
apply review
Browse files Browse the repository at this point in the history
  • Loading branch information
@hamza1311
hamza1311 committed Aug 14, 2022
1 parent 0107f03 commit 1eea41a
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 69 deletions.
43 changes: 21 additions & 22 deletions packages/yew-macro/src/html_tree/html_element.rs
View file
Expand Up @@ -8,7 +8,7 @@ use syn::spanned::Spanned;
use syn::{Block, Expr, Ident, Lit, LitStr, Token};

use super::{HtmlChildrenTree, HtmlDashedName, TagTokens};
use crate::props::{ClassesForm, ElementProps, Prop};
use crate::props::{ClassesForm, ElementProps, Prop, PropDirective};
use crate::stringify::{Stringify, Value};
use crate::{non_capitalized_ascii, Peek, PeekValue};

Expand Down Expand Up @@ -139,21 +139,21 @@ impl ToTokens for HtmlElement {
|Prop {
label,
value,
is_forced_attribute,
directive,
..
}| {
(
label.to_lit_str(),
value.optimize_literals_tagged(),
*is_forced_attribute,
*directive,
)
},
);
let boolean_attrs = booleans.iter().filter_map(
|Prop {
label,
value,
is_forced_attribute,
directive,
..
}| {
let key = label.to_lit_str();
Expand Down Expand Up @@ -183,7 +183,7 @@ impl ToTokens for HtmlElement {
},
),
},
*is_forced_attribute,
*directive,
))
},
);
Expand Down Expand Up @@ -215,7 +215,7 @@ impl ToTokens for HtmlElement {
__yew_classes
}
}),
false,
None,
))
}
ClassesForm::Single(classes) => {
Expand All @@ -227,7 +227,7 @@ impl ToTokens for HtmlElement {
Some((
LitStr::new("class", lit.span()),
Value::Static(quote! { #lit }),
false,
None,
))
}
}
Expand All @@ -237,26 +237,29 @@ impl ToTokens for HtmlElement {
Value::Dynamic(quote! {
::std::convert::Into::<::yew::html::Classes>::into(#classes)
}),
false,
None,
))
}
}
}
});

fn apply_as(directive: Option<&PropDirective>) -> TokenStream {
match directive {
Some(PropDirective::ApplyAsProperty(token)) => quote_spanned!(token.span()=> ::yew::virtual_dom::ApplyAttributeAs::Property),
None => quote!(::yew::virtual_dom::ApplyAttributeAs::Attribute),
}
}

/// Try to turn attribute list into a `::yew::virtual_dom::Attributes::Static`
fn try_into_static(src: &[(LitStr, Value, bool)]) -> Option<TokenStream> {
fn try_into_static(src: &[(LitStr, Value, Option<PropDirective>)]) -> Option<TokenStream> {
let mut kv = Vec::with_capacity(src.len());
for (k, v, is_forced_attribute) in src.iter() {
for (k, v, directive) in src.iter() {
let v = match v {
Value::Static(v) => quote! { #v },
Value::Dynamic(_) => return None,
};
let apply_as = if *is_forced_attribute {
quote! { ::yew::virtual_dom::ApplyAttributeAs::Attribute }
} else {
quote! { ::yew::virtual_dom::ApplyAttributeAs::Property }
};
let apply_as = apply_as(directive.as_ref());
kv.push(quote! { ( #k, #v, #apply_as ) });
}

Expand All @@ -266,15 +269,11 @@ impl ToTokens for HtmlElement {
let attrs = normal_attrs
.chain(boolean_attrs)
.chain(class_attr)
.collect::<Vec<(LitStr, Value, bool)>>();
.collect::<Vec<(LitStr, Value, Option<PropDirective>)>>();
try_into_static(&attrs).unwrap_or_else(|| {
let keys = attrs.iter().map(|(k, ..)| quote! { #k });
let values = attrs.iter().map(|(_, v, is_forced_attribute)| {
let apply_as = if *is_forced_attribute {
quote! { ::yew::virtual_dom::ApplyAttributeAs::Attribute }
} else {
quote! { ::yew::virtual_dom::ApplyAttributeAs::Property }
};
let values = attrs.iter().map(|(_, v, directive)| {
let apply_as = apply_as(directive.as_ref());
let value = wrap_attr_value(v);
quote! { ::std::option::Option::map(#value, |it| (it, #apply_as)) }
});
Expand Down
27 changes: 16 additions & 11 deletions packages/yew-macro/src/props/prop.rs
View file
Expand Up @@ -13,19 +13,24 @@ use super::CHILDREN_LABEL;
use crate::html_tree::HtmlDashedName;
use crate::stringify::Stringify;

#[derive(Copy, Clone)]
pub enum PropDirective {
ApplyAsProperty(Token![~]),
}

pub struct Prop {
pub is_forced_attribute: bool,
pub directive: Option<PropDirective>,
pub label: HtmlDashedName,
/// Punctuation between `label` and `value`.
pub value: Expr,
}
impl Parse for Prop {
fn parse(input: ParseStream) -> syn::Result<Self> {
let at = input.parse::<Token![@]>().map(|_| true).unwrap_or(false);
let directive = input.parse::<Token![~]>().map(|parsed| PropDirective::ApplyAsProperty(parsed)).ok();
if input.peek(Brace) {
Self::parse_shorthand_prop_assignment(input, at)
Self::parse_shorthand_prop_assignment(input, directive)
} else {
Self::parse_prop_assignment(input, at)
Self::parse_prop_assignment(input, directive)
}
}
}
Expand All @@ -37,7 +42,7 @@ impl Prop {
/// an ambiguity in the syntax
fn parse_shorthand_prop_assignment(
input: ParseStream,
is_forced_attribute: bool,
directive: Option<PropDirective>,
) -> syn::Result<Self> {
let value;
let _brace = braced!(value in input);
Expand Down Expand Up @@ -67,12 +72,12 @@ impl Prop {
Ok(Self {
label,
value: expr,
is_forced_attribute,
directive,
})
}

/// Parse a prop of the form `label={value}`
fn parse_prop_assignment(input: ParseStream, is_forced_attribute: bool) -> syn::Result<Self> {
fn parse_prop_assignment(input: ParseStream, directive: Option<PropDirective>) -> syn::Result<Self> {
let label = input.parse::<HtmlDashedName>()?;
let equals = input.parse::<Token![=]>().map_err(|_| {
syn::Error::new_spanned(
Expand All @@ -95,7 +100,7 @@ impl Prop {
Ok(Self {
label,
value,
is_forced_attribute,
directive,
})
}
}
Expand All @@ -118,10 +123,10 @@ fn parse_prop_value(input: &ParseBuffer) -> syn::Result<Expr> {

match &expr {
Expr::Lit(_) => Ok(expr),
_ => Err(syn::Error::new_spanned(
ref exp => Err(syn::Error::new_spanned(
&expr,
"the property value must be either a literal or enclosed in braces. Consider \
adding braces around your expression.",
format!("the property value must be either a literal or enclosed in braces. Consider \
adding braces around your expression.: {:#?}", exp),
)),
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/yew-macro/src/props/prop_macro.rs
View file
Expand Up @@ -64,7 +64,7 @@ impl From<PropValue> for Prop {
Prop {
label,
value,
is_forced_attribute: false,
directive: None,
}
}
}
Expand Down
42 changes: 13 additions & 29 deletions packages/yew/src/dom_bundle/btag/attributes.rs
View file
Expand Up @@ -145,8 +145,7 @@ impl Attributes {
Some(old) => old != new,
None => true,
} {
// todo: set property
el.set_attribute(k, new.0).unwrap();
Self::set(&el, k, new.0, new.1);
}
}

Expand All @@ -164,16 +163,9 @@ impl Attributes {
el.set_attribute(key, value).expect("invalid attribute key")
}
ApplyAttributeAs::Property => {
match key {
// need to be attributes because, otherwise query selectors fail
"class" => el.set_attribute(key, value).expect("invalid attribute key"),
_ => {
let key = JsValue::from_str(key);
let value = JsValue::from_str(value);
js_sys::Reflect::set(el.as_ref(), &key, &value)
.expect("could not set property");
}
}
let key = JsValue::from_str(key);
let value = JsValue::from_str(value);
js_sys::Reflect::set(el.as_ref(), &key, &value).expect("could not set property");
}
}
}
Expand All @@ -184,17 +176,9 @@ impl Attributes {
.remove_attribute(key)
.expect("could not remove attribute"),
ApplyAttributeAs::Property => {
match key {
// need to be attributes because, otherwise query selectors fail
"class" => el
.remove_attribute(key)
.expect("could not remove attribute"),
_ => {
let key = JsValue::from_str(key);
js_sys::Reflect::set(el.as_ref(), &key, &JsValue::UNDEFINED)
.expect("could not remove property");
}
}
let key = JsValue::from_str(key);
js_sys::Reflect::set(el.as_ref(), &key, &JsValue::UNDEFINED)
.expect("could not remove property");
}
}
}
Expand Down Expand Up @@ -375,23 +359,23 @@ mod tests {
async fn macro_syntax_works() {
#[function_component]
fn Comp() -> Html {
html! { <a href="https://example.com/" @alt="abc" /> }
html! { <a href="https://example.com/" ~alt="abc" /> }
}

let output = gloo::utils::document().get_element_by_id("output").unwrap();
yew::Renderer::<Comp>::with_root(output.clone()).render();

gloo::timers::future::sleep(Duration::from_secs(1)).await;
let element = output.query_selector("a").unwrap().unwrap();
assert_eq!(element.get_attribute("alt").unwrap(), "abc");
assert_eq!(element.get_attribute("href").unwrap(), "https://example.com/");

assert_eq!(
Reflect::get(element.as_ref(), &JsValue::from_str("href"))
.expect("no href")
Reflect::get(element.as_ref(), &JsValue::from_str("alt"))
.expect("no alt")
.as_string()
.expect("not a string"),
"https://example.com/",
"property `href` not set properly"
"abc",
"property `alt` not set properly"
);
}
}
4 changes: 2 additions & 2 deletions packages/yew/src/virtual_dom/mod.rs
View file
Expand Up @@ -242,7 +242,7 @@ impl From<IndexMap<AttrValue, AttrValue>> for Attributes {
fn from(map: IndexMap<AttrValue, AttrValue>) -> Self {
let v = map
.into_iter()
.map(|(k, v)| (k, (v, ApplyAttributeAs::Property)))
.map(|(k, v)| (k, (v, ApplyAttributeAs::Attribute)))
.collect();
Self::IndexMap(v)
}
Expand All @@ -252,7 +252,7 @@ impl From<IndexMap<&'static str, AttrValue>> for Attributes {
fn from(v: IndexMap<&'static str, AttrValue>) -> Self {
let v = v
.into_iter()
.map(|(k, v)| (AttrValue::Static(k), (v, ApplyAttributeAs::Property)))
.map(|(k, v)| (AttrValue::Static(k), (v, ApplyAttributeAs::Attribute)))
.collect();
Self::IndexMap(v)
}
Expand Down
7 changes: 3 additions & 4 deletions packages/yew/src/virtual_dom/vtag.rs
View file
Expand Up @@ -369,11 +369,10 @@ impl VTag {
);
}

/// Adds a key-value pair to element's property
/// Set the given key as property on the element
///
/// Not everything works when it set as an property. We use workarounds for:
/// `class`, which is set as attribute.
pub fn set_property(&mut self, key: &'static str, value: impl Into<AttrValue>) {
/// [`js_sys::Reflect`] is used for setting properties.
pub fn add_property(&mut self, key: &'static str, value: impl Into<AttrValue>) {
self.attributes.get_mut_index_map().insert(
AttrValue::Static(key),
(value.into(), ApplyAttributeAs::Property),
Expand Down

0 comments on commit 1eea41a

Please sign in to comment.