Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Feat:add webview inspector #1443

Merged
merged 10 commits into from
Jul 4, 2020
Merged

Feat:add webview inspector #1443

merged 10 commits into from
Jul 4, 2020

Conversation

wswebcreation
Copy link
Contributor

@wswebcreation wswebcreation commented Jun 29, 2020 •
edited
Loading

This PR will add a webview inspector for Android AND iOS with:

  • A select box for selecting the webview.
    image
  • Tooltip for the select box
    image
  • webview being shown with the title of the page, to make it easier to select the correct webview (in case of multiple). This was working out of the box for iOS but not for Android. So that has been added
    image
  • when you select a webview it will show a proper HTML-tree, head/script are removed
    image
  • proper logic to select elements by adding wdith/height/x/y data on the HTML
    image
  • select elements when the address bar has been made smaller due to manual swiping
    image
  • add a warning to notify people that the Appium-Desktop webview inspector might not be that accurate
    image
  • it will automatically start in the webview-context if a session for a Chrome or Safari browser is started (removed the default which went to NATIVE_APP)
  • it will automatically start in the webview-context if a session for a Hybrid app is started with autoWebview: true
  • A pain point for a lot of users is that Appium (Desktop) breaks if they don't provide the correct ChromeDriver version. I now let Appium Desktop download it by default so AD will support Chrome and Android Hybrid apps out of the box without any errors by enabling the relaxed security flag by default. A tooltip is added with the reason why.
    image
  • when a browser session is started AD will automatically load a default url, which will be the AD docs. This is done because ChromeDriver starts with an empty page and that breaks the logic because there is no HTML to show and users get stuck

@KazuCocoa
Copy link
Member

yey, see this pr tomorrow :)

@jlipps
Copy link
Member

jlipps commented Jun 29, 2020

don't have time to review just yet, but wow, looks amazing!

@wswebcreation
Copy link
Contributor Author

wswebcreation commented Jun 30, 2020 •
edited
Loading

@KazuCocoa and @jlipps

In this piece of code https://github.com/appium/appium-desktop/blob/master/app/main/appium.js#L233 we start Appium-Desktop in the native context by default. Can we disable this when this PR is getting merged?

KazuCocoa
KazuCocoa reviewed Jun 30, 2020
View reviewed changes
app/main/appium-method-handler.js Outdated Show resolved Hide resolved
app/main/appium-method-handler.js Outdated Show resolved Hide resolved
app/main/appium-method-handler.js Outdated Show resolved Hide resolved
app/main/appium-method-handler.js Show resolved Hide resolved
app/renderer/actions/Inspector.js Outdated Show resolved Hide resolved
@KazuCocoa
Copy link
Member

In this piece of code https://github.com/appium/appium-desktop/blob/master/app/main/appium.js#L233 we start Appium-Desktop in the native context by default. Can we disable this when this PR is getting merged?

Yes, I think we can remove there.
The reason was this AD had not supported web context. By this change, the situation will change.

@wswebcreation
Copy link
Contributor Author

In this piece of code https://github.com/appium/appium-desktop/blob/master/app/main/appium.js#L233 we start Appium-Desktop in the native context by default. Can we disable this when this PR is getting merged?

Yes, I think we can remove there.
The reason was this AD had not supported web context. By this change, the situation will change.

@KazuCocoa

Yes, that did the trick, tnx

@wswebcreation
feat: add Android support
33bdcad 
- automatically download chromedriver
- refactor get source order
- refactor helpers and add webview helpers
- refactor parsing HTML source
- default iOS url
@wswebcreation
feat: add start in correct context
771c9b3 
- remove starting in native context for browsers and allow using autoWebview property
- add NATIVE_APP const
- fix object deconstruction issue with iOS
- add start browser with initial url for iOS and Android
@wswebcreation
Copy link
Contributor Author

@jlipps @dpgraham @KazuCocoa

I've finished the PR and removed the draft text. All added features are explained in the description and it works for Android and iOS.

Thanks for the review so far and please let me know if I need to do more for it.

@wswebcreation
chore: code cleanup
3cc4e17 
- clean up adjusted code
- fix all linting warnings in non touched files
KazuCocoa
KazuCocoa reviewed Jul 1, 2020
View reviewed changes
app/main/appium.js Outdated Show resolved Hide resolved
app/main/webviewHelpers.js Show resolved Hide resolved
app/renderer/actions/Session.js Show resolved Hide resolved
@wswebcreation
feat: auto enable relaxed security
b4abc2c 
- remove default auto chrome download for allowInsecure prop
- fixed key for context selectbox tooltip
- added tooltip to relaxed security checkbox
KazuCocoa
KazuCocoa approved these changes Jul 3, 2020
View reviewed changes
Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

Left a minor comment, otherwise lgtm

app/main/appium-method-handler.js Outdated Show resolved Hide resolved
@KazuCocoa KazuCocoa merged commit 0df71bf into appium:master Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KazuCocoa KazuCocoa KazuCocoa approved these changes

@dpgraham dpgraham Awaiting requested review from dpgraham

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wswebcreation @KazuCocoa @jlipps