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

feat: Implement new window requested event, closes #527 #526

Merged

Conversation

atlanticaccent
Copy link
Contributor

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

The WKWebview implementation is outstanding but I'm giving it a shot.
Also couldn't test this properly as whilst testing using WSL I couldn't get popup windows to work.

Regardless of the above, these "naïve" implementations may allow for new popup windows to bypass/ignore existing webview settings. For example, they do not inherit parent window navigation handlers for example. This isn't a new thing - it's just more noticeable when implementing a feature like this where we have functions that can cancel events.

@atlanticaccent atlanticaccent requested review from a team March 23, 2022 21:31
@atlanticaccent atlanticaccent changed the title Implement new window requested event feat: Implement new window requested event, closes #527 Mar 24, 2022
@amrbashir amrbashir linked an issue Mar 24, 2022 that may be closed by this pull request
2 tasks
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Windows, and Linux implementation is LGTM. There is some small nit-picks and I will leave reviewing the macOS to @wusyong

examples/new_window_req_event.rs Outdated Show resolved Hide resolved
src/webview/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@keiya01 keiya01 left a comment

Choose a reason for hiding this comment

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

It's working great on macos.
it looks good to me.

@amrbashir amrbashir requested a review from wusyong June 18, 2022 18:13
wusyong
wusyong previously approved these changes Jun 19, 2022
@atlanticaccent atlanticaccent requested a review from a team as a code owner June 19, 2022 10:10
amrbashir
amrbashir previously approved these changes Jun 19, 2022
@amrbashir amrbashir merged commit fa5456c into tauri-apps:dev Jun 19, 2022
@github-actions github-actions bot mentioned this pull request Jun 19, 2022
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.

Add new window requested event callback/event
4 participants