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.

Sign up for GitHub

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

Jump to bottom

fix: adjust highlighters for Android tablets #1162

Merged
merged 1 commit into from
Oct 31, 2023
Merged

fix: adjust highlighters for Android tablets #1162

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/renderer/lib/appium-client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,13 @@ export default class AppiumClient {
if (_.toLower(platformName) === 'android' && _.toLower(automationName) === 'uiautomator2') {
// returned Android height and width can both be affected by UiAutomator2 calculations
// we stick with device dimensions, but swap them depending on detected orientation
// deviceScreenSize value fits portrait mode for phones, but landscape mode for tablets
const [width, height] = deviceScreenSize.split('x');
if (windowSize.height > windowSize.width) { // portrait mode
if ((windowSize.height > windowSize.width && height > width) || // phone portrait mode
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be >= instead of > ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's definitely an edgecase. If the dimensions are equal for both windowSize and deviceScreenSize, then either branch is fine. The only way problems could arise is if width == height, but windowSize.height != windowSize.width. I'm not sure how likely this is to actually happen.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also finding this condition a bit hard to read. Would it make more sense to reverse it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created #1163 to adjust this

(windowSize.height < windowSize.width && height < width)) { // tablet landscape mode
windowSize.height = height;
windowSize.width = width;
} else { // landscape mode
} else { // phone landscape mode / tablet portrait mode
windowSize.height = width;
windowSize.width = height;
}
Expand Down