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

Add Android WebView #1017

Merged
merged 3 commits into from
Aug 15, 2020
Merged

Conversation

paulproteus
Copy link
Contributor

Add Android WebView

Here's a screenshot of the app running the new WebView demo.

image

Notable missing features:

  • No onload. It's too bad, but I can't make it work without rubicon-java subclassing.
  • No on key down. Doesn't seem that important, but I'm OK adding it. I'd rather get this merged, and deal with that in a follow-up pull request, if possible.

If there are code style problems, I'd prefer to have you fix them if you're OK with it @freakboy3742 . I'm open to making revisions, just scared of latency.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Feature wise, this looks great. The lack of an onload handler is a little annoying, but what is here is good enough for demo purposes, no worse than it is on other platforms.

We should capture what we know about the onload problem as a bug report. If we know what the solution will look like when rubicon-java is up to the task, it would be good to have that written down so we can make the fix (or even have a prototype implementation present in the source, commented out).

Other than that, the only thing preventing me from merging is the sizing of the widget itself. The webview is definitely filling the screen - but it looks to me like it is overfilling. If you scroll to the bottom of the page, the scroll bar disappears, which suggests that the widget is bigger than the visible area. This may be the screen size/layout issue we've been tracking; now is as good a time as any to put that one to bed.

@paulproteus
Copy link
Contributor Author

We should capture what we know about the onload problem as a bug report. If we know what the solution will look like when rubicon-java is up to the task, it would be good to have that written down so we can make the fix (or even have a prototype implementation present in the source, commented out).

Makes sense. I filed #1020 since I prefer not having dead code in the repo, but if you prefer that way, I am open to that.

Other than that, the only thing preventing me from merging is the sizing of the widget itself. The webview is definitely filling the screen - but it looks to me like it is overfilling. If you scroll to the bottom of the page, the scroll bar disappears, which suggests that the widget is bigger than the visible area. This may be the screen size/layout issue we've been tracking; now is as good a time as any to put that one to bed.

Aw man... I agree in theory, but I also think that the layout issue is going to take more investigating, so I really want to lobby for you to let this get merged, and then for us to fix the layout issue in a follow-up.

Let me know if you're possibly convinceable. :)

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Ok; I can accept the layout problem is a different problem; I'll merge this as is, and put the layout bug on the todo list.

@freakboy3742 freakboy3742 merged commit 9f772c2 into beeware:master Aug 15, 2020
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