report: allow custom strings for throttling/device #11796

paulirish · 2020-12-09T17:16:23Z

fixes #7053
also addresses's brendan's review on #11876

(i guess this is technically not breaking. \o/)

Impl is mostly adding the new properties so the strings are exposed. But also, as patrick called out (https://github.com/GoogleChrome/lighthouse/pull/11779/files/f3c6d3528eab9cdd1b5c0978eafe3b617e0fc45a#r538013675) the logic for building the "Device" Runtime Settings string didn't make sense anymore, so that's changed.

cc @benschwarz

lighthouse-core/config/experimental-config.js

@@ -13,6 +13,11 @@
 /** @type {LH.Config.Json} */
 const config = {
  extends: 'lighthouse:default',
+  settings: {


lighthouse-core/report/html/renderer/util.js

@@ -424,12 +424,23 @@ class Util {
        networkThrottling = Util.i18n.strings.runtimeUnknown;
    }

-    const deviceEmulation = settings.formFactor === 'mobile'
+    let device = !settings.screenEmulation ? Util.i18n.strings.runtimeNoEmulation :


connorjclark · 2020-12-09T17:37:00Z

lighthouse-core/report/html/renderer/util.js

-  /** Descriptive explanation for emulation setting when emulating a Moto G4 mobile device. */
-  runtimeMobileEmulation: 'Emulated Moto G4',
+  /** Descriptive explanation for emulation setting when emulating a mobile device. */
+  runtimeMobileEmulation: 'Emulated Mobile',


You want to stop saying Moto G4?

Yes. The settings they provide just tell us mobile or no. What sort of mobile device is determined by the UA. So yeah we gotta be more general with this tring.

This reverts commit 6cb6812.

lighthouse-core/test/report/html/renderer/util-test.js

+      }
+    `);
+  });
+


lighthouse-core/test/report/html/renderer/util-test.js

+    clonedSampleResult.configSettings.providedNetworkThrottlingString = 'Packet-level throtting';
+    clonedSampleResult.configSettings.providedCPUThrottlingString = 'cgroups cpu throttling';
+
+    const descs = opts => Util.getEmulationDescriptions(opts);


lighthouse-core/report/html/renderer/util.js

@@ -66,6 +66,7 @@ class Util {
    if (!clone.configSettings.locale) {
      clone.configSettings.locale = 'en';
    }
+    // If LHR is older than v7, it uses emulatedFormFactor instead of formFactor. https://github.com/GoogleChrome/lighthouse/blob/master/docs/emulation.md#changes-made-in-v7


connorjclark

lgtm

brendankenny

these will also automatically work as CLI flags, so should probably document them unless you purposefully didn't want them documented there :)

brendankenny · 2021-01-13T00:13:20Z

types/externs.d.ts

@@ -175,6 +175,12 @@ declare global {
      throttlingMethod?: 'devtools'|'simulate'|'provided';
      /** The throttling config settings. */
      throttling?: ThrottlingSettings;
+      /** A user-facing string to display in Runtime Settings for the Device row */


sorry to bikeshed, but we are going to be stuck with these indefinitely, so... 😬

Exploring the possibility space:

should these be nested under an object of provided override strings? Keeps them all in one place, allows for a home for future override strings without things getting ridiculous, etc

didn't we eventually decide "provided" is confusing, since it's not always clear who is providing what? What about something like override*? display*?

brendankenny · 2021-01-13T00:14:59Z

types/externs.d.ts

+      /** A user-facing string to display in Runtime Settings for the Network throttling row */
+      providedNetworkThrottlingString?: string | null;
+      /** A user-facing string to display in Runtime Settings for the CPU throttling row */
+      providedCPUThrottlingString?: string | null;


we've stuck to

Suggested change

providedCPUThrottlingString?: string | null;

providedCpuThrottlingString?: string | null;

style for all the other settings with acronyms/initialisms

…actor-providedstrings

brendankenny

LGTM!

brendankenny · 2021-02-24T23:34:31Z

lighthouse-core/config/constants.js

@@ -103,6 +103,8 @@ const defaultSettings = {
  screenEmulation: screenEmulationMetrics.mobile,
  emulatedUserAgent: userAgents.mobile,

+  displayStrings: {},


not the prettiest thing to default to {}, but after discussion we think maybe this is the best compromise given the constraints of the Settings type today. It would be kind of nice to default to not defined if there are no strings in it, but semantically I think it's still ok (the empty set of display strings)

brendankenny · 2021-02-24T23:36:48Z

types/config.d.ts

@@ -151,6 +151,7 @@ declare global {
      export interface Settings extends Required<SharedFlagsSettings> {
        throttling: Required<ThrottlingSettings>;
        screenEmulation: ScreenEmulationSettings;
+        displayStrings: DisplayStrings;


nit: is this necessary to set manually? It should already be set to this from the base Required<SharedFlagsSettings>, I believe...

brendankenny · 2021-02-24T23:40:49Z

types/externs.d.ts

@@ -176,6 +185,8 @@ declare global {
      throttlingMethod?: 'devtools'|'simulate'|'provided';
      /** The throttling config settings. */
      throttling?: ThrottlingSettings;
+      /* Optional object of user-facing strings to be used instead of Lighthouse's given values. */
+      displayStrings?: DisplayStrings;


seeing it now, lhr.configSettings.displayStringsisn't the clearest that it's not the equivalent ofrendererFormattedStrings`, its user defined displayStrings, but I'm also ok with it if you like the name :)

…actor-providedstrings

paulirish · 2022-04-25T21:31:32Z

This implementation is pretty stale now. And it's a low priority feature. The UI is wildly different now.

Ben/others, if you still really want this, holler and we'll figure something out.

benschwarz · 2022-04-26T00:22:30Z

@paulirish ok to close from my perspective. we don't output using lighthouse html, though I imagine some other downstream users of LH might. can always be reopened in the future! thanks y'all.

paulirish requested a review from a team as a code owner December 9, 2020 17:16

paulirish requested review from connorjclark and removed request for a team December 9, 2020 17:16

google-cla bot added the cla: yes label Dec 9, 2020

paulirish mentioned this pull request Dec 9, 2020

core(emulation): refactor emulation settings & CLI flags #11779

Merged

devtools-bot assigned connorjclark Dec 9, 2020

devtools-bot added the waiting4reviewer label Dec 9, 2020

connorjclark reviewed Dec 9, 2020

View reviewed changes

paulirish added the 7.1 label Dec 14, 2020

Base automatically changed from emulation-flags-refactor to master December 16, 2020 01:24

paulirish added 8 commits December 22, 2020 14:34

report: support v6 settings

6cb6812

Revert "report: support v6 settings"

3ec4a83

This reverts commit 6cb6812.

brendan's suggestion

c7171ef

connor's suggestion

e41bb6c

report: allow custom strings for throttling/device

fe10677

initialize as null

cc7f575

test expectation

599fa22

incorporate #11876

b806ea9

paulirish force-pushed the emulation-flags-refactor-providedstrings branch from 2f148a0 to b806ea9 Compare December 23, 2020 00:06

vercel bot deployed to Preview December 23, 2020 00:06 View deployment

drop changes to experimental config

18b041c

vercel bot deployed to Preview December 23, 2020 00:08 View deployment

Merge branch 'master' into emulation-flags-refactor-providedstrings

bb1dda6

vercel bot deployed to Preview December 23, 2020 01:21 View deployment

cli snapshots

5e17723

vercel bot deployed to Preview December 23, 2020 01:22 View deployment

comments for brendan

4ed9b08

vercel bot deployed to Preview December 23, 2020 01:26 View deployment

connorjclark reviewed Jan 11, 2021

View reviewed changes

paulirish mentioned this pull request Jan 12, 2021

report: convert v6 emulatedFormFactor to v7 formFactor #11876

Merged

feedback

2983ffa

vercel bot deployed to Preview January 12, 2021 23:21 View deployment

connorjclark approved these changes Jan 12, 2021

View reviewed changes

connorjclark added waiting4committer and removed waiting4reviewer labels Jan 12, 2021

brendankenny reviewed Jan 13, 2021

View reviewed changes

paulirish added the P1 label Feb 11, 2021

paulirish self-assigned this Feb 11, 2021

paulirish removed the 7.1 label Feb 11, 2021

paulirish added 3 commits February 11, 2021 11:16

Merge remote-tracking branch 'origin/master' into emulation-flags-ref…

a153556

…actor-providedstrings

to object

a3b89fc

apply object to types and elsewhere

ad980a7

vercel bot deployed to Preview February 24, 2021 22:53 View deployment

paulirish assigned brendankenny Feb 24, 2021

paulirish added waiting4reviewer and removed waiting4committer labels Feb 24, 2021

empty obj instead of | undefined

28bb3a6

vercel bot deployed to Preview February 24, 2021 23:14 View deployment

brendankenny approved these changes Feb 24, 2021

View reviewed changes

paulirish added 2 commits February 24, 2021 16:35

nit

377fa6f

Merge remote-tracking branch 'origin/master' into emulation-flags-ref…

be0de3d

…actor-providedstrings

vercel bot deployed to Preview February 25, 2021 00:36 View deployment

connorjclark added waiting4committer and removed waiting4reviewer labels Apr 8, 2021

paulirish closed this Apr 25, 2022

paulirish deleted the emulation-flags-refactor-providedstrings branch April 25, 2022 21:32

benschwarz mentioned this pull request Nov 29, 2022

Report shows "Emulated Moto G4" when testing on a mobile device #14560

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report: allow custom strings for throttling/device #11796

report: allow custom strings for throttling/device #11796

paulirish commented Dec 9, 2020 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

connorjclark Dec 9, 2020

paulirish Dec 23, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

connorjclark left a comment

brendankenny left a comment

brendankenny Jan 13, 2021 •

edited

Loading

brendankenny Jan 13, 2021

brendankenny left a comment

brendankenny Feb 24, 2021

brendankenny Feb 24, 2021

brendankenny Feb 24, 2021

paulirish commented Apr 25, 2022

benschwarz commented Apr 26, 2022

	providedCPUThrottlingString?: string \| null;
	providedCpuThrottlingString?: string \| null;

report: allow custom strings for throttling/device #11796

report: allow custom strings for throttling/device #11796

Conversation

paulirish commented Dec 9, 2020 • edited Loading

This comment was marked as resolved.

This comment was marked as resolved.

connorjclark Dec 9, 2020

Choose a reason for hiding this comment

paulirish Dec 23, 2020

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

connorjclark left a comment

Choose a reason for hiding this comment

brendankenny left a comment

Choose a reason for hiding this comment

brendankenny Jan 13, 2021 • edited Loading

Choose a reason for hiding this comment

brendankenny Jan 13, 2021

Choose a reason for hiding this comment

brendankenny left a comment

Choose a reason for hiding this comment

brendankenny Feb 24, 2021

Choose a reason for hiding this comment

brendankenny Feb 24, 2021

Choose a reason for hiding this comment

brendankenny Feb 24, 2021

Choose a reason for hiding this comment

paulirish commented Apr 25, 2022

benschwarz commented Apr 26, 2022

paulirish commented Dec 9, 2020 •

edited

Loading

brendankenny Jan 13, 2021 •

edited

Loading