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

added support for window.open in onCreateWindow (android) #1679

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zopagaduanjr
Copy link

@zopagaduanjr zopagaduanjr commented May 31, 2023

Connection with issue(s)

Resolve issue #1633

related to issue #755

Testing and Review Notes

Currently on Android, onCreateWindow's createWindowAction returns a null url if open.window() function is used.

createWindowAction will only return a non-null url for anchor tags with href specified.

Upon further inspection, it seems that webview's onCreateWindow couldn't retrieve url value if its not an anchor tag.

Hence I've proposed a support for open.window() flow, utilizing its HitTestResult type of unknown type. Aside from the additional logic if type is HitTestResult.UKNOWN_TYPE, this PR also updates the onCreateWindow parameters. isDialog and isUserGesture is made final so that inner class could access it.

Here's a gist to test the behavior of onCreateWindow Url:

https://gist.github.com/zopagaduanjr/b119f5f06d4d3af7bdcbb3b7f99b61ed

Screenshots or Videos

video demo

Screen.Recording.2023-06-01.at.1.48.43.AM.mov

onCreateWindow Urls

Screenshot 2023-06-01 at 1 49 13 AM

To Do

  • double check the original issue to confirm it is fully satisfied
  • add testing notes and screenshots in PR description to help guide reviewers
  • request the "UX" team perform a design review (if/when applicable)

@Vis5
Copy link

Vis5 commented Jun 29, 2023

please approve this PR :)

@littlesmilelove
Copy link

This feature is much needed, thanks! @pichillilorenzo

@akhilesh-sharma-GSY
Copy link

Please merge this PR if everything is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants