Skip to content

Seed app taking over some navigation on page #473

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

Closed
SHDavies opened this issue Jun 1, 2020 · 7 comments
Closed

Seed app taking over some navigation on page #473

SHDavies opened this issue Jun 1, 2020 · 7 comments

Comments

@SHDavies
Copy link

SHDavies commented Jun 1, 2020

I may not be using the seed API correctly, but I am experiencing a bug while including a seed app in an existing page. When clicking most of the links on the existing page, the address in my browser's address bar is updated, but the new URL isn't actually navigated to. Curiously, a few links on the page still work, and I haven't been able to discern what sets them apart. My app does not handle any routing, and I have built the app with this code:

fn after_mount(_url: Url, orders: &mut impl Orders<Msg>) -> AfterMount<Model> {
    ...
    AfterMount::new(model).url_handling(UrlHandling::None)
}

#[wasm_bindgen(start)]
pub fn render() {
    App::builder(update, view)
        .after_mount(after_mount)
        .routes(|_| None)
        .build_and_start();
}

I'm on seed v0.6 if that makes any difference. I'm hoping I'm just missing something in the docs, but at this point I'm stumped.

@MartinKavik
Copy link
Member

  1. I recommend to use master or at least this Seed "version": https://github.com/seed-rs/seed-quickstart/blob/master/Cargo.toml#L22. I'm rewriting docs for 0.7.0+ from scratch and it's not ready yet, but all examples are updated to the latest API. API is simpler and there is no routes, window_events, GMsg, sink and similar necessary complex stuff. There is a list of examples with routing - Suggestions for Url-Api #433 (comment) (please ignore context in this issue).
  2. See link handling logic: https://github.com/seed-rs/seed/blob/master/src/browser/service/routing.rs#L154-L169 - maybe it helps.
  3. Please write examples of your working and not working links.

@SHDavies
Copy link
Author

SHDavies commented Jun 2, 2020

I am on the latest version of seed now, and through logging I've narrowed the problem down to these lines:

notify(Notification::new(subs::UrlRequested(
url.clone(),
subs::url_requested::UrlRequest::new(
subs::url_requested::UrlRequestStatus::default(),
Some(event.clone()),
),
)));

I haven't been able to follow exactly what is happening yet (I'm going to keep working on it later), but at some point in that code, prevent_default is being called on the event.

@SHDavies
Copy link
Author

SHDavies commented Jun 2, 2020

I've also determined that on the links that are working, for some reason the event target is an unrelated <img>. I'm wondering if this is a problem with web-sys though, and I'll worry about it later once I get these other links working.

@MartinKavik
Copy link
Member

I think I understand your problem now.

When clicking most of the links on the existing page, the address in my browser's address bar is updated, but the new URL isn't actually navigated to.

It's not bug, it's feature. Seed intercepts all links and in some cases (e.g. a element + href + relative url) prevents default behavior (go to chosen location and refresh page) because in 99% of cases you don't want to kill and reload the app. So this behavior isn't "opt-in" but default and "opt-out". You can disable it:

fn init(url: Url, orders: &mut impl Orders<Msg>) -> Model {
    orders
        .subscribe(|subs::UrlRequested(_, url_request)| url_request.handled())

Docs - UrlRequest methods

I can describe you how link handling works, however it'll be written in official seed-rs.org docs in a few weeks.

Please make sure you test on the current Seed master so we can debug the same code.
I tested it on todomvc example.


I've also determined that on the links that are working, for some reason the event target is an unrelated .

If your img is inside a with relative url in href and click on it causes redirect, then it looks like a bug. If so, please post a part of your HTML with a and img with ideally original links.
Thanks.

@SHDavies
Copy link
Author

SHDavies commented Jun 2, 2020

That code snippet has solved the problem. Perhaps it will make more sense once this is all fully documented, but it seems odd to me that seed would preventDefault on all links on a page, including ones it doesn't have route handling logic for, especially when that's the case that this code seems to be handling:

if let Some(routes) = routes {
if let Some(redirect_msg) = routes(url.clone()) {
// Route internally, overriding the default history
push_route(url);
event.prevent_default(); // Prevent page refresh
update(redirect_msg);
}
}

Again, I may just be missing some context here that would help this make sense.

I may have a bit more insight into what's happening with the <img> tag being the event target. This is roughly the HTML before a link is clicked on:

<ul id="systemAdminListCollapsible">
    <li class="clickable"><img src="/img/minus.gif">Menu
        <ul class="systemAdminlistitem">
            <li><a href="/example_1">Example 1</a></li>
            <li><a href="/example_2">Example 2</a></li>
            <li><a href="/example_3">Example 3</a></li>
            <li><a href="/example_4">Example 4</a></li>
            <li><a href="/example_5">Example 5</a></li>
            <li><a href="/example_6">Example 6</a></li>
        </ul>
    </li>
</ul>

and after a link gets clicked on, some other JS lib is changing the menu item into this:

<ul id="systemAdminListCollapsible">
    <li class="clickable"><img src="/img/plus.gif" id="THIS IS THE EVENT TARGET">Menu
        <ul class="systemAdminlistitem" style="display: none;">
            <li><a href="/example_1">Example 1</a></li>
            <li><a href="/example_2">Example 2</a></li>
            <li><a href="/example_3">Example 3</a></li>
            <li><a href="/example_4">Example 4</a></li>
            <li><a href="/example_5">Example 5</a></li>
            <li><a href="/example_6">Example 6</a></li>
        </ul>
    </li>
</ul>

The link is still in the DOM, but perhaps being set to display: none is causing some weird behavior.

@MartinKavik
Copy link
Member

this code seems to be handling

That code is a deprecated part of the old API - the important code is now:

notify(Notification::new(subs::UrlRequested(
url.clone(),
subs::url_requested::UrlRequest::new(
subs::url_requested::UrlRequestStatus::default(),
Some(event.clone()),
),
)));

Then there is Seed's internal subs::UrlRequested subscriber, that is invoked after user's subscribers (e.g. your snippet to disable Seed's default link handling) and if the user hasn't changed UrlRequest status to handled, then Seed's internal subscriber prevents default browser behavior, pushes url to the browser history and fires subs::UrlChanged.

on all links on a page, including ones it doesn't have route handling logic for

I don't know what do you mean by "doesn't have route handling logic for" - there is basically no router in Seed => you just listen for UrlChanged and update Model accordingly. It's not important if UrlChanged has been fired because of link click, direct browser url change by the user or your app fired UrlRequested to simulate link click. So Seed has to listen for all events and you decide what to do. It's anti-pattern to reload the app on click on relative link so the default behavior prevents this.

a link gets clicked on, some other JS lib is changing the menu item into this

In general, front-end frameworks don't like modifying DOM by other libraries because it often breaks VDOM patching - it's possible that your library accidentally forces Seed to patch/recreate links or it removes link interceptors. The safest integration of JS libs is to leverage Web Components (custom elements).

I hope it makes sense a bit, I'll try to write it better in the docs.

@SHDavies
Copy link
Author

SHDavies commented Jun 2, 2020

Thanks, that clears things up a bit. I got hung up on that deprecated code. What I meant by "doesn't have route handling logic for" is just if the routes function returns None, meaning it's a url your app doesn't care about. But if the routes function is deprecated, then it's a moot point. Thanks for your help!

@SHDavies SHDavies closed this as completed Jun 2, 2020
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

No branches or pull requests

2 participants