Skip to content

Conversation

@connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Jan 13, 2023

ref #14256

@connorjclark connorjclark requested a review from a team as a code owner January 13, 2023 01:36
@connorjclark connorjclark requested review from brendankenny and removed request for a team January 13, 2023 01:36
@adamraine
Copy link
Contributor

We're also going to need to update DT settings when this is rolled since DT sets up emulation on it's own

Copy link
Contributor

@adamraine adamraine left a comment

Choose a reason for hiding this comment

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

Should we update the user agent as well?

copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Feb 2, 2023
Lighthouse plans to use the Moto G Power as the default emulated device
for version 10.0. This will require the Moto G Power to be in the
emulated device list when running Lighthouse in DevTools.

Lighthouse PR:
GoogleChrome/lighthouse#14674

Bug: None
Change-Id: I2335bf8bc933b84fe13f85ea585ffb7e5c7ee506
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4214256
Commit-Queue: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
Reviewed-by: Mathias Bynens <[email protected]>
copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Feb 3, 2023
This should accompany the release of Lighthouse 10.0 which switches the
default mobile device to the Moto G power.

Lighthouse PR:
GoogleChrome/lighthouse#14674

Bug: None
Change-Id: I84154e10181d9c02e32daa70893b7ff4ce5905aa
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4219345
Reviewed-by: Connor Clark <[email protected]>
Commit-Queue: Connor Clark <[email protected]>
url: 'http://localhost:10200/dobetterweb/lighthouse-1024x680.jpg?iar1',
},
length: 1,
length: 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the new one, should we add an expectation for it now?

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 unrelated, just so happened that the other audit being tested here got caught up. Don't wanna add it here...cuz it is named specific to its audit... :/

"throttlingMethod": "simulate",
"screenEmulation": {
"mobile": true,
"width": 360,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably good for this PR, but we should do another complete artifact refresh to capture any changes the screen adjustment had.

expect(oppElements.map(e => e.id).sort()).toEqual(oppAudits.map(a => a.id).sort());
expect(oppElements.length).toBeGreaterThan(0);
expect(oppElements.length).toMatchInlineSnapshot('7');
expect(oppElements.length).toMatchInlineSnapshot('6');
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the change here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm sure it was just image aspect ratio or responsive images no longer failing according to the artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants