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

Embedding Android WebView doc looks broken #884

Open
brodycj opened this issue Sep 12, 2018 · 12 comments
Open

Embedding Android WebView doc looks broken #884

brodycj opened this issue Sep 12, 2018 · 12 comments

Comments

@brodycj
Copy link
Contributor

brodycj commented Sep 12, 2018

Issue Type
[x] Bug
[ ] Feature Request

Priority
[ ] Minor
[x] Major
[ ] Critical
[ ] Blocker

Environment

  • OS: N/A
  • Browser: N/A

Description

In https://cordova.apache.org/docs/en/latest/guide/platforms/android/webview.html:

Sample code seems to use a Config object class.

The one place where I see a Config class documented is in: https://developer.android.com/reference/android/util/Config

where I see no init method documented.

As a general rule the document should never assume the user implicitly knows that s/he has to import a class from a package somewhere.

This looks broken to me, hoping to see an explanation and solution someday soon.

P.S. android.util.Config class looks deprecated according to https://developer.android.com/reference/android/util/Config.

@brodycj
Copy link
Contributor Author

brodycj commented Nov 13, 2018

@martinhorvath commented in apache/cordova-android#494 (comment):

I [...] tried to reflect my findings in this update: apache/cordova-docs@6dc8836

Can somebody check if this is useful for others?

A pull request would be really appreciated.

@martinhorvath
Copy link

I had to find out that I really messed up the conventions of the commit and didn't want to create an even bigger mess with a pull request. Should I?

@janpio
Copy link
Member

janpio commented Nov 13, 2018

I honestly don't understand how you were able to commit to this repo and where this commit actually "lives". How did you do that @martinhorvath? Which branch did you commit to?

@dpogue
Copy link
Member

dpogue commented Nov 13, 2018

@janpio The commit is in his fork, it's just a GitHub URL/UI thing that makes it appear like it's in apache/cordova-docs.

@martinhorvath
Copy link

It indeed is... I thought "lets see what happens if I suggest an edit using the url from the offical docs". So.... what shall I do? :-)

@brodycj
Copy link
Contributor Author

brodycj commented Nov 13, 2018

@martinhorvath a pull request would be much easier for us to track and comment on than a loose commit. While we would prefer the proposed commits to be cleaner your proposed commit would be better than nothing.

@janpio
Copy link
Member

janpio commented Nov 13, 2018

Got it: martinhorvath@6dc8836

@janpio
Copy link
Member

janpio commented Nov 13, 2018

You can just create a Pull Request into this repository from your patch-1 branch. Start here @martinhorvath: https://github.com/apache/cordova-docs/compare

@brodycj
Copy link
Contributor Author

brodycj commented Nov 13, 2018

And we can always use git show -b 6dc8836 or git diff -b to ignore the space changes.

@martinhorvath
Copy link

Ok let's see if I can fix this...

martinhorvath added a commit to martinhorvath/cordova-docs that referenced this issue Nov 13, 2018
The documentation of embedding cordova in native apps is outdated and this updated is considered as a quick update.
@martinhorvath
Copy link

PR done, I tried my best to follow the contribution guidelines. Would appreciate if you let me know what I missed to do.
--> #904

martinhorvath added a commit to martinhorvath/cordova-docs that referenced this issue Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants