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

Adding id fields to most components. #10

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

Conversation

quinnjr
Copy link

@quinnjr quinnjr commented Oct 29, 2020

Summary

This pull request adds id fields to most of the Property structs of each component of the library and associated additions of the id field into the html templates of the corresponding component.

Example:

#[derive(Clone, Debug, Properties, PartialEq)]
pub struct ColumnsProps {
    ...
    #[prop_or_default]
    pub id: String, // New addition of `id` field.
    ...
}

impl Component for Columns {
    ...
    type Properties = ColumnsProps;
    ...
    fn view(&self) -> Html {
        ...
        let id = &self.props.id;
        html! {
            <div class=classes id=id>
              {self.props.children.clone()}
            </div>
        }
    }
}

NOTE: The id field was already present for Modals and was thus left untouched.

The id field was added to each *Props struct below the pub classes: Option<String> field with #[prop_or_default] enabled on each id addition.

Why so many ids?

My current legacy code that I am converting from Angular to Yew contains work that used ids on components, which I expected to be able to use on ybc elements. I felt it would be beneficial to add the field across the library in a standardized way for those who expect the html attribute to be present.

As well, using ids in vanilla javascript, such as:

const el = document.getElementById(id);

is much easier to work with inlined javascript.

Finally, idis also a part of the global attributes defined on all html elements per the HTML Living Standard and I felt it was appropriate to define the attribute as common on all components in this library.

Contribution Checklist

  • Executed cargo clippy and cargo fmt on code.
  • Updated Cargo.toml version field.
  • Updated CHANGELOG.md.
  • Updated Release page.

@thedodd
Copy link
Owner

thedodd commented Oct 30, 2020

Hey @quinnjr, thanks for taking the time to open this PR! It is on my shortlist to get it reviewed and merged.

@quinnjr
Copy link
Author

quinnjr commented Oct 30, 2020

@thedodd thank you! Also, when you have a chance, can you re-run the CI tasks on the pull request? Looks like Github had some network issues while trying to test my commit for fixing the cargo fmt error.

@thedodd
Copy link
Owner

thedodd commented Feb 15, 2023

@quinnjr hello there (3 years later). If you are still interested in getting this work landed, we have a set of beta releases being published for the 0.4.0 line (Yew 0.20 and all). I would be happy to get this work landed and released as part of the beta if you are still interested.

@quinnjr
Copy link
Author

quinnjr commented Feb 21, 2023

I moved back to using Angular instead of Yew. I'm not much interested in working on this, unfortunately.

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.

2 participants