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

CHNL-13255 Add wrapper view for hosting form webview #196

Closed

Conversation

dan-peluso
Copy link
Contributor

@dan-peluso dan-peluso commented Nov 4, 2024

Description

Creates a dialog we can wrap the webview we'll interact with around. I have a video showing the functionality below, but note this PR does not address the following open issues.

The main difference between this and the last feature add is that we can now click outside the view and it will close the dialog.

  • How do we determine if the content has fully loaded? Onsite team knows about this and are coming up with a way to communicate that, for now we're using the documentReady method which klaviyo.js does not respect.
  • How do we communicate a button press closing the dialog? Our current bridge functionality looks for the view to be removed from the tree, which explains why the dialog closes after the webview has already been collapsed. We need a way of hooking into the button press action rather than the removal of the button id from the tree. I ended up using the close event we send as part of klaviyo.js. Much faster close times here!
webview-as-dialog.webm

using the klaviyo.js "close" event:

Check List

  • Are you changing anything with the public API?
  • Are your changes backwards compatible with previous SDK Versions?
  • Have you tested this change on real device?
  • Have you added unit test coverage for your changes?
  • Have you verified that your changes are compatible with all Android versions the SDK currently supports?
    ^ we might have an issue supporting devices below Android API 17 but our SDK does not support those anyway.

Changelog / Code Overview

  • making a dialog for the webview, and extrapolating the webview client code into its own class for better separation of intents

Test Plan

Related Issues/Tickets

CHNL-13255
CHNL-13258
CHNL-13259
CHNL-13256

@dan-peluso dan-peluso requested a review from a team as a code owner November 4, 2024 21:24
@dan-peluso dan-peluso requested review from kennyklaviyo and removed request for a team November 4, 2024 21:24
@dan-peluso
Copy link
Contributor Author

We opted to not do this since we can launch a fullscreen webview much easier than handling the wrapper, and the styling looks great on it

@dan-peluso dan-peluso closed this Nov 7, 2024
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.

4 participants