thedodd / ybc

A Yew component library based on the Bulma CSS framework.
Apache License 2.0
258 stars 37 forks source link

Adding `id` fields to most components. #10

Open quinnjr opened 3 years ago

quinnjr commented 3 years ago

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

thedodd commented 3 years ago

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

quinnjr commented 3 years ago

@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 commented 1 year ago

@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 commented 1 year ago

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