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

Seed shouldn't intercept links with target attribute #497

Open
MartinKavik opened this issue Jul 15, 2020 · 7 comments
Open

Seed shouldn't intercept links with target attribute #497

MartinKavik opened this issue Jul 15, 2020 · 7 comments
Labels
help wanted Extra attention is needed missing functionality Not quite a bug, but needs to be addressed

Comments

@MartinKavik
Copy link
Member

MartinKavik commented Jul 15, 2020

See

// @TODO: https://github.com/elm/browser/blob/9f52d88b424dd12cab391195d5b090dd4639c3b0/src/Elm/Kernel/Browser.js#L157

Should we ignore all links with set target or only with some target values (e.g. _self)?

MDN docs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a


Workaround:

orders.subscribe(|subs::UrlRequested(url, url_request)| {
    if should_not_intercept_this_url(url) {
        url_request.handled()
   }
});
@MartinKavik MartinKavik added help wanted Extra attention is needed missing functionality Not quite a bug, but needs to be addressed labels Jul 15, 2020
@MartinKavik MartinKavik removed the 0.8.0 label Sep 10, 2020
@arn-the-long-beard
Copy link
Member

Hey, why should we ignore it ?

Here is the quote from the doc :

target
Where to display the linked URL, as the name for a browsing context (a tab, window, or <iframe>). The following keywords have special meanings for where to load the URL:

    _self: the current browsing context. (Default)
    _blank: usually a new tab, but users can configure browsers to open a new window instead.
    _parent: the parent browsing context of the current one. If no parent, behaves as _self.
    _top: the topmost browsing context (the "highest" context that’s an ancestor of the current one). If no ancestors, behaves as _self.

@MartinKavik
Copy link
Member Author

When you write attrs!{ At::Target => "_blank" }, then you hope the link will be opened in a new tab. However Seed doesn't ignores it (if the link has a relative URL) and handles it as a regular link / route => it behaves exactly like _self instead of _blank.

P.S. There are also other exceptions that should be handled - e.g. when the user holds Control key during the click, it should open a new tab, but it doesn't happen because Seed handles it as a regular link click.

@mankinskin
Copy link

But why can't it open a new tab?

@MartinKavik
Copy link
Member Author

But why can't it open a new tab?

Seed intercepts all relative links by default to prevent the app reloading. The intercepted link click doesn't propagate to the browser's handler so it doesn't process the link at all in such cases.
Taking things like target="_blank" into account is basically a missing Seed feature.

@mankinskin
Copy link

mankinskin commented Feb 16, 2021

Ah, okay. Now I understand. I wonder what should happen with the other target values. Does seed have a concept for these "browsing contexts"? None that I am aware of. A simple solution to this problem might be to just stop intercepting any relative link calls with target=_blank but the other target values would still not be handled correctly.

But maybe that is just fine? I am not sure what a "browsing context" could be other than a browser page or an iframe. So I guess to handle target properly, seed needs to provide some story for tabs and iframes.

@MartinKavik
Copy link
Member Author

MartinKavik commented Feb 16, 2021

I would just handle the most common cases - it means Ctrl key + click and target="_blank". We can add special handling for other cases when somebody needs it - I would try to respect YAGNI.

Does seed have a concept for these "browsing contexts"? None that I am aware of.

There aren't any special concepts / apis for "browsing contexts".

@mankinskin
Copy link

Makes sense. I guess anything the other targets do can be emulated using other features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed missing functionality Not quite a bug, but needs to be addressed
Projects
None yet
Development

No branches or pull requests

3 participants