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

Better styling for router related components #477

Merged
merged 7 commits into from
Feb 6, 2023

Conversation

Threated
Copy link
Contributor

@Threated Threated commented Feb 6, 2023

Made a few changes to IntoAttribute to make it usable as Box<dyn IntoAttribute> to support styling similar to native tags.

Before the <A> tag took a MaybeSignal<String> so you needed to do:

view!{cx,
  <A href="home" class="my-class".to_string()>
  <A href="home" class=Signal::derive(cx, move || "my-class".to_string())>
}

Now you can do:

view!{cx,
  <A href="home" class="my-class">
  <A href="home" class=move || "my-class">
}

And it also works for Form, ActionForm and MultiActionForm

I tried really hard to implement this using generics, but it does not seem possible. The type of the generic parameter needs to be inferred even if its value is None because it needs to know what Option::<T>::None it is.

@gbj
Copy link
Collaborator

gbj commented Feb 6, 2023

Oh I like this a lot. I should probably do the same thing for class, properties, and views.

I think we should export a type alias for this to make it easier to use. Something like AttributeValue? Do you have thoughts on naming?

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

AttributeValue sounds good to me. Where should we I put the alias? In leptos/src/lib.rs to the children type aliases?

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

Ouh I have something to make it even better!

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

Btw is it intended that I have to say class=class and can't just say class in the attributes?
If i do <h1 class></h1> the macro will resolve to .attr("class",(cx, "") instead of .attr("class", (cx, class)).

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

You have to love Rust for this conversion trait magic 🪄
I implemented IntoAttribute for (Scope, Option<AttributeValue>) and (Scope, AttributeValue) so they can work directly in the view Macro.

This works now:

#[component]
pub fn MyHeading(
  cx: Scope,
  text: String,
  #[prop(optional, into)]
  class: Option<AttributeValue>
) -> impl IntoView {
  view!{
    cx,
    <h1 class=class>{text}</h1>
  }
}

@gbj
Copy link
Collaborator

gbj commented Feb 6, 2023

Btw is it intended that I have to say class=class and can't just say class in the attributes?

If i do <h1 class></h1> the macro will resolve to .attr("class",(cx, "") instead of .attr("class", (cx, class)).

On this particular point, this is just to maintain compatibility with the way Boolean attributes work in HTML.

<button disabled> in HTML is the same as <button disabled="">

I may add a shorthand syntax in the view macro like <button {disabled}> to expand to <button disabled=disabled> in the future, but not sure if that's a good idea yet.

@gbj
Copy link
Collaborator

gbj commented Feb 6, 2023

AttributeValue sounds good to me. Where should we I put the alias? In leptos/src/lib.rs to the children type aliases?

Yeah that's what I was thinking.

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

On this particular point, this is just to maintain compatibility with the way Boolean attributes work in HTML.

Ah I see that makes sense.

I may add a shorthand syntax in the view macro like <button {disabled}> to expand to <button disabled=disabled> in the future, but not sure if that's a good idea yet.

Yeah thats debatable. I was just a bit confused as I expected it to work because the syntax works for custom components.

@Threated
Copy link
Contributor Author

Threated commented Feb 6, 2023

Should I change anything or is this ready to merge?

@gbj
Copy link
Collaborator

gbj commented Feb 6, 2023

Nope looks good! Merging now.

@gbj gbj merged commit b0a98d8 into leptos-rs:main Feb 6, 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