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

The desktop app will default open link in browser #133

Merged
merged 10 commits into from
Jan 19, 2022

Conversation

mrxiaozhuox
Copy link
Contributor

No description provided.

@mrxiaozhuox
Copy link
Contributor Author

wait please, I will add a remove-browser-open attribute

@mrxiaozhuox
Copy link
Contributor Author

mrxiaozhuox commented Jan 18, 2022

I use browser-href to replace the href, because I think we don't any way prevent <a> redirect. So defulat I will change the href to the #

Copy link
Member

@jkelleyrtp jkelleyrtp left a comment

Choose a reason for hiding this comment

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

This is a good start, but I'd like to see this code moved into the event handler instead of setattribute.

We use listener multiplexing - so there's only one click handler for the whole app. Adding more handlers to the app manually might interfere with our global handling.

Instead of having this code here, we need it to either go in the Rust side where we receive events from webview, or in the AddEventListener code where we create the global listeners.

Additionally, instead of the "browser-open" attribute, we should just look at the URL itself and make the decision automatically. If it starts with a protocol (ie wry:// or dioxus:// - or anything that's not http:// or https://) then we want the url to move through the handler infrastructure.

Also - any onclick in Desktop should have "default prevented" - that way we don't need to mess with window.location. By calling "prevent default", the history will be preserved properly, so we can still provide links into a desktop app from the outside.

To recap:

  • Move rpc.call into the "add event listener"
  • Always call "prevent-default" for onclick events in desktop
  • Parse URL or "prevent_default" attribute instead of "browser-open" flag

packages/desktop/src/index.js Outdated Show resolved Hide resolved
packages/desktop/src/lib.rs Outdated Show resolved Hide resolved
packages/desktop/src/lib.rs Outdated Show resolved Hide resolved
examples/link.rs Outdated Show resolved Hide resolved
Copy link
Member

@jkelleyrtp jkelleyrtp left a comment

Choose a reason for hiding this comment

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

Add a check that href isn't null/undefined and then remove the console.log and we're good to go :)

packages/desktop/src/index.js Outdated Show resolved Hide resolved
@jkelleyrtp
Copy link
Member

Looks great :) Thank you!

@jkelleyrtp jkelleyrtp merged commit 887f69d into DioxusLabs:master Jan 19, 2022
jkelleyrtp pushed a commit that referenced this pull request Jun 28, 2023
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