-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
feat: enhance hybrid mode controls #1006
Conversation
} from '@ant-design/icons'; | ||
|
||
const HYBRID_MODE_DOCS_URL = 'https://github.com/appium/appium/blob/1.x/docs/en/writing-running-appium/web/hybrid.md'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlipps Might this doc be outdated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we need the link, maybe https://appium.github.io/appium.io/docs/en/writing-running-appium/web/hybrid/ will be better. but afaik the documentation is outdated. (ideally, each driver that supports hybrid need something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now changed the link to the appium.github.io version, I agree that it looks better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could link to https://appium.io/docs/en/2.0/guides/context/, and then link from this page to driver-specific context docs as necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm more than happy to update the docs links (including others elsewhere in the code), but I would prefer to keep the current links until the new docs are updated with more information. Right now the context page content is rather abstract and could use more concrete information on webview contexts (and links to driver-specific documentation as well). I would also suggest having (Appium-maintained) client examples like in the old docs.
assets/locales/en/translation.json
Outdated
"usingSwitchContextRecommended": "Webview context(s) detected; certain elements might only be accessible after switching to a webview context, which must be done in an Appium script. See http://appium.io/docs/en/writing-running-appium/web/hybrid/ for more information", | ||
"usingWebviewContext": "Using the Webview inspector in Appium Inspector is less accurate in retrieving and selecting DOM elements in comparison to using the DevTools of Chrome and Safari.", | ||
"contextSwitcher": "You can switch to a different context here. See http://appium.io/docs/en/writing-running-appium/web/hybrid/ for more information", | ||
"noAdditionalContextsFound": "Appium has detected only the native context", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to put my 2 cents there: It would probably be too generic or not even correct to say "Appium has detected" as Appium is just an abstract server. Webview detection is done by the corresponding driver, for example UIA2 or XCUITest one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. I decided that instead of explicitly mentioning 'Appium driver' or something similar, it would still be fine to use even more generic wording 😄
} from '@ant-design/icons'; | ||
|
||
const HYBRID_MODE_DOCS_URL = 'https://github.com/appium/appium/blob/1.x/docs/en/writing-running-appium/web/hybrid.md'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we need the link, maybe https://appium.github.io/appium.io/docs/en/writing-running-appium/web/hybrid/ will be better. but afaik the documentation is outdated. (ideally, each driver that supports hybrid need something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 this is a huge improvement!
* feat: add info element if no other contexts found * feat: add context switcher in app header * chore: remove context info under selected element * fix: update info string and docs link
This PR enhances the UX for the Inspector's hybrid mode with two main changes:
hybrid.mp4
One small caveat is that the documentation link for hybrid mode now points to the 1.x docs in the GitHub repo, since this info is not yet available in the new docs. It's still an improvement over the old link, which leads to a 404.