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

ordering of classes different when reordering or re-adding #926

Closed
1 of 3 tasks
liquidnya opened this issue Feb 6, 2020 · 2 comments · Fixed by #927 or #1085
Closed
1 of 3 tasks

ordering of classes different when reordering or re-adding #926

liquidnya opened this issue Feb 6, 2020 · 2 comments · Fixed by #927 or #1085
Labels

Comments

@liquidnya
Copy link
Contributor

Problem
If changing the order of classes on an element (or re-adding classes to an element) does not reflect the specified order in the DOM of the browser.

Steps To Reproduce
Steps to reproduce the behavior:

  1. display the following Html in a view:
html! {<div class="class-1 class-2 class-3">{ "Hello, World!" }</div>}
  1. change the order of the classes in the view, after an update occured:
html! {<div class="class-3 class-2 class-1">{ "Hello, World!" }</div>}
  1. inspect the class of the element in the browser:
<div class="class-1 class-2 class-3">Hello, World!</div>

Expected behavior
After the update of the order of the classes i expected the following:

<div class="class-3 class-2 class-1">Hello, World!</div>

Test Code

use yew::{html, Callback, ClickEvent, Component, ComponentLink, Html, ShouldRender};

struct App {
    classes: Vec<&'static str>,
    onclick: Callback<ClickEvent>,
}

enum Msg {
    Click,
}

impl Component for App {
    type Message = Msg;
    type Properties = ();

    fn create(_: Self::Properties, link: ComponentLink<Self>) -> Self {
        App {
            classes: vec!["class-1", "class-2", "class-3"],
            onclick: link.callback(|_| Msg::Click),
        }
    }

    fn update(&mut self, msg: Self::Message) -> ShouldRender {
        match msg {
            Msg::Click => {
                self.classes.reverse();
                true
            }
        }
    }

    fn view(&self) -> Html {
        let class = self.classes.join(" ");
        html! {
            <>
                <button onclick=&self.onclick>{ "Toggle class order!" }</button>
                <div class=&class>{ "Hello, World!" }</div>
                <div>{ &class }</div>
            </>
        }
    }
}

fn main() {
    yew::start_app::<App>();
}

This is the result when starting this with cargo web start and clicking the button once:
grafik

Steps To Reproduce when re-adding classes

When re-adding classes the order is also lost:

  1. display the following Html in a view:
html! {<div class="class-1 class-2 class-3">{ "Hello, World!" }</div>}
  1. remove class-2 in the view, after an update occured:
html! {<div class="class-3 class-1">{ "Hello, World!" }</div>}
  1. re-add class-2 in the view, after an update occured:
html! {<div class="class-1 class-2 class-3">{ "Hello, World!" }</div>}
  1. inspect the class of the element in the browser:
<div class="class-1 class-3 class-2">Hello, World!</div>

Expected behavior when re-adding classes
After re-adding class-2, i expected the following:

<div class="class-1 class-2 class-3">Hello, World!</div>

Environment:

  • Yew version 0.11.0
  • Rust version rustc 1.41.0 (5e1a79984 2020-01-27)
  • stdweb v0.4.20
  • cargo-web 0.6.25
  • OS: Windows 10
  • Browser Firefox
  • Browser version 72.0.2 (64-Bit)

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@liquidnya liquidnya added the bug label Feb 6, 2020
@jstarry
Copy link
Member

jstarry commented Feb 6, 2020

Thanks for the issue! And thanks for the interest in fixing this, it's much appreciated 👍

Here's the relevant code: https://github.com/yewstack/yew/blob/master/src/virtual_dom/vtag.rs#L198

Hope this helps!

@liquidnya
Copy link
Contributor Author

The issue is directly related to diff_classes and stdweb's TokenList (or rather the underlying DOMTokenList).
stdweb only gives access to the class_list() (classList) and not the class property (className) itself and only the following methods are supported: len, add, remove, contains.

I would like to suggest the following solution:
diff_classes computes, not only the differences between the ancestor and self, but rather returns a sequence of add and remove patches, when applied to the TokenList (class_list()) results into the list representing the VTags classes set.

E.g. diff_classes for the classes sets ["class-3", "class-2", "class-1"] (self) and ["class-1", "class-2", "class-3"] (ancestor)
returns the following patches: [Patch::Remove("class-1"), Patch::Remove("class-2"), Patch::Add("class-2", ()), Patch::Add("class-1", ())]

I will try implementing this approach, since it only affects the implementation of diff_classes.

liquidnya pushed a commit to liquidnya/yew that referenced this issue Feb 7, 2020
liquidnya pushed a commit to liquidnya/yew that referenced this issue Feb 7, 2020
liquidnya pushed a commit to liquidnya/yew that referenced this issue Feb 7, 2020
jstarry pushed a commit that referenced this issue Feb 8, 2020
…ront (#927)

* Fix wrong order of classes when reordering or adding classes in the front/middle (#926)

* Fix coding style (#926)

* Use set_attribute(class, className) for classes (#926)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants