Skip to content

/ playwright

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

Jump to bottom

Missing a few device defs for landscape variants #606

Closed
pdehaan opened this issue Jan 23, 2020 · 3 comments
Closed

Missing a few device defs for landscape variants #606

pdehaan opened this issue Jan 23, 2020 · 3 comments
Labels
bug good first issue

Comments

@pdehaan
Copy link

@pdehaan pdehaan commented Jan 23, 2020

Randomly noticed that a few items didn't specify a "landscape" device:

Missing device for "iPhone 11 landscape"
Missing device for "iPhone 11 Pro landscape"
Missing device for "iPhone 11 Pro Max landscape"
Missing device for "Microsoft Lumia 550 landscape"

... So I hacked a quick linter to verify:

const devices = require("playwright").devices;

const deviceMap = new Map();
for (const device of devices) {
  deviceMap.set(device.name, device);
}

for (const name of deviceMap.keys()) {
  if (name.endsWith(" landscape")) continue;
  const landscapeName = `${name} landscape`;
  if (!deviceMap.has(landscapeName)) {
    const landscapeDevice = deviceMap.get(name);
    const {width, height} = landscapeDevice.viewport;
    landscapeDevice.name = landscapeName;
    landscapeDevice.viewport.width = height;
    landscapeDevice.viewport.height = width;
    console.log(`Missing device for "${landscapeName}", expected: ${JSON.stringify(landscapeDevice, null, 2)}`);
  } else {
    const portrait = deviceMap.get(name);
    const landscape = deviceMap.get(landscapeName);
    if (!(portrait.width === landscape.height && portrait.height === landscape.width)) {
      console.log(`${name} viewport ({width:${portrait.viewport.width},height:${portrait.viewport.height}}) does not match ${landscapeName} viewport ({width:${landscape.viewport.width},height=${landscape.viewport.height}})`);
    }
  }
}

OUTPUT

Missing device for "iPhone 11 landscape", expected:
{
  "name": "iPhone 11 landscape",
  "userAgent": "Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Mobile/15E148 Safari/604.1",
  "viewport": {
    "width": 896,
    "height": 414,
    "deviceScaleFactor": 2,
    "isMobile": true
  }
}

Missing device for "iPhone 11 Pro landscape", expected:
{
  "name": "iPhone 11 Pro landscape",
  "userAgent": "Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Mobile/15E148 Safari/604.1",
  "viewport": {
    "width": 812,
    "height": 375,
    "deviceScaleFactor": 3,
    "isMobile": true
  }
}

Missing device for "iPhone 11 Pro Max landscape", expected:
{
  "name": "iPhone 11 Pro Max landscape",
  "userAgent": "Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Mobile/15E148 Safari/604.1",
  "viewport": {
    "width": 896,
    "height": 414,
    "deviceScaleFactor": 3,
    "isMobile": true
  }
}

Missing device for "Microsoft Lumia 550 landscape", expected:
{
  "name": "Microsoft Lumia 550 landscape",
  "userAgent": "Mozilla/5.0 (Windows Phone 10.0; Android 4.2.1; Microsoft; Lumia 550) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Mobile Safari/537.36 Edge/14.14263",
  "viewport": {
    "width": 360,
    "height": 640,
    "deviceScaleFactor": 2,
    "isMobile": true
  }
}

Although, that "Lumia 550" definition looks possibly suspicious.

playwright/src/deviceDescriptors.ts

Lines 491 to 500 in b4209e9

{
'name': 'Microsoft Lumia 550',
'userAgent': 'Mozilla/5.0 (Windows Phone 10.0; Android 4.2.1; Microsoft; Lumia 550) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Mobile Safari/537.36 Edge/14.14263',
'viewport': {
'width': 640,
'height': 360,
'deviceScaleFactor': 2,
'isMobile': true
}
},

It looks like the width > height, so it's possibly already the landscape definition, but incorrectly labeled (and we're missing the portrait definition).
@JoelEinbinder

This comment has been minimized.

Sign in to view
Copy link
Contributor

@JoelEinbinder JoelEinbinder commented Jan 23, 2020

Thanks for digging into this!
@sotayamashita

This comment has been minimized.

Sign in to view
Copy link
Contributor

@sotayamashita sotayamashita commented Jan 24, 2020

Can I work on?
@aslushnikov

This comment has been minimized.

Sign in to view
Copy link
Contributor

@aslushnikov aslushnikov commented Jan 28, 2020

@sotayamashita please do!
@sotayamashita sotayamashita mentioned this issue Jan 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@pdehaan @JoelEinbinder @sotayamashita @aslushnikov
You can’t perform that action at this time.