report: convert v6 emulatedFormFactor to v7 formFactor #11876
Conversation
| @@ -424,10 +424,24 @@ class Util { | |||
| networkThrottling = Util.i18n.strings.runtimeUnknown; | |||
| } | |||
|
|
|||
| // TODO(paulirish): revise Runtime Settings strings: https://github.com/GoogleChrome/lighthouse/pull/11796 | |||
| const deviceEmulation = settings.formFactor === 'mobile' | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| : Util.i18n.strings.runtimeDesktopEmulation; | ||
|
|
||
| let deviceEmulation; | ||
| // COMPAT: remove this fallback handling at Lighthouse v8 |
There was a problem hiding this comment.
Why remove in v8? Don't we want to keep the renderer backwards-compat indefinitely?
There was a problem hiding this comment.
Oh, right, there's three possibilities for the old way...
There was a problem hiding this comment.
deviceEmulation = {
mobile: Util.i18n.strings.runtimeMobileEmulation,
desktop: Util.i18n.strings.runtimeDesktopEmulation,
none: Util.i18n.strings.runtimeNoEmulation,
}[settings.formFactor || settings.emulatedFormFactor || 'none']maybe?
or
deviceEmulation = {
mobile: Util.i18n.strings.runtimeMobileEmulation,
desktop: Util.i18n.strings.runtimeDesktopEmulation,
}[settings.formFactor || settings.emulatedFormFactor] || Util.i18n.strings.runtimeNoEmulation
|
can we upgrade the LHR to the expected format in |
This reverts commit 6cb6812.
connorjclark
changed the title
| @@ -66,6 +66,10 @@ class Util { | |||
| if (!clone.configSettings.locale) { | |||
| clone.configSettings.locale = 'en'; | |||
| } | |||
| if (!clone.configSettings.formFactor) { | |||
| // @ts-expect-error fallback handling for emulatedFormFactor | |||
There was a problem hiding this comment.
can you add a version comment like on the other comments? Someday we won't remember when this change happened :)
| const deviceEmulation = settings.formFactor === 'mobile' | ||
| ? Util.i18n.strings.runtimeMobileEmulation | ||
| : Util.i18n.strings.runtimeDesktopEmulation; | ||
| const deviceEmulation = { |
There was a problem hiding this comment.
The fallback case shouldn't ever happen for the 'mobile'|'desktop' type it is today, so probably worth adding a comment about why there's a fallback to runtimeNoEmulation
|
Well, you were too fast, but maybe in the arbitrary strings PR :) edit from paul since github isn't automatically crosslinking...... yup i addressed in #11796 (comment) |
fixes #11873
restored the handling for the report's "Device" string for v6.
I redid the conditionals using ternaries, but that bit has the same logic.