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

Add Other variant to the ListenerKind #2417

Merged
merged 3 commits into from Jan 31, 2022

Conversation

XX
Copy link
Contributor

@XX XX commented Jan 27, 2022

Description

This pull request provides a new listener kind variant so that user-defined listeners can be added to the VTag. The new ListenerKind::other(Cow<'static, str>) variant makes possible to implement the Listener trait for a user-defined listener type with a custom event name.

This option is very useful for the custom Html builders. These are temporary and only used in view so they can't use the gloo::events::EventListener. For example, have the following custom event listener:

pub struct EventListener<E> {
    event_type: &'static str,
    callback: Callback<E>,
}

impl<E> Listener for EventListener<E>
where
    E: From<JsValue> + Clone,
{
    fn kind(&self) -> ListenerKind {
        ListenerKind::other(Cow::Borrowed(self.event_type))
    }

    fn handle(&self, event: Event) {
        let event: E = JsValue::from(event).into();
        self.callback.emit(event);
    }

    fn passive(&self) -> bool {
        false
    }
}

And the MDC widget TopAppBar builder using it:

pub struct TopAppBar {
    html: Html,
}

impl TopAppBar {
    pub fn new() -> Self {
        Self {
            html: html! {
                <header class = "mdc-top-app-bar" data-mdc-auto-init = "MDCTopAppBar">
                    <div class = "mdc-top-app-bar__row">
                        <section class = "mdc-top-app-bar__section mdc-top-app-bar__section--align-start">
                            <span class = "mdc-top-app-bar__title"></span>
                        </section>
                        <section class = "mdc-top-app-bar__section mdc-top-app-bar__section--align-end"
                                role = "toolbar">
                        </section>
                    </div>
                </header>
            },
        }
    }
    
    ...
    
    pub fn on_navigation(self, callback: impl Into<Callback<Event>>) -> Self {
        self.listener(Rc::new(EventListener::new("MDCTopAppBar:nav", callback.into())))
    }
}

It sets the listener for the event with name "MDCTopAppBar:nav" inside view:

fn view(&self, _ctx: &Context<Self>) -> Html {
    let drawer = self.create_drawer("app-drawer");

    let top_app_bar = TopAppBar::new()
        .id("top-app-bar")
        .title("App title")
        .navigation_item(IconButton::new().icon("menu"))
        .enable_shadow_when_scroll_window()
        .on_navigation(|_| {
            let drawer = dom::existing::get_element_by_id::<Element>("app-drawer")
                .get(drawer::mdc::TYPE_NAME);
            let opened = drawer.get("open").as_bool().unwrap_or(false);
            drawer.set("open", !opened);
        });

    html! {
        <>
            { drawer }
            <div class = "mdc-drawer-scrim"></div>

            <div class = { classes!("app-content", Drawer::APP_CONTENT_CLASS) }>
                { top_app_bar }

                <div class = { TopAppBar::FIXED_ADJUST_CLASS }>
                   ...
                </div>
            </div>
        </>
    }
}

The full working example: https://github.com/noogen-projects/yew-mdc-widgets/blob/master/examples/client/src/main.rs

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests

@github-actions
Copy link

Visit the preview URL for this PR (updated for commit 3f992d8):

https://yew-rs-api--pr2417-other-listener-kind-1itcdlyy.web.app

(expires Thu, 03 Feb 2022 19:46:05 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@voidpumpkin voidpumpkin added the A-yew Area: The main yew crate label Jan 30, 2022
Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine. I don't remember when Listener made this change but I think it used to allow that.

I ran into this problem before and my solution was far more uglier.


Related to #1777

@hamza1311 hamza1311 added this to the v0.20 milestone Jan 30, 2022
#[allow(non_camel_case_types)]
#[allow(missing_docs)]
pub enum ListenerKind {
$( $kind, )*
other(std::borrow::Cow<'static, str>),
Copy link
Member

@siku2 siku2 Jan 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should use proper style for this variant and use upper case Other. This also conveys that it's different from the other variants.

But given that we're dealing with a special enum, this isn't a blocking issue.

@voidpumpkin voidpumpkin merged commit d5cc4a3 into yewstack:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants