From e3dfef3f6396b0b8804c5aaa8b7aa681217bc15f Mon Sep 17 00:00:00 2001 From: Gabrielle Langer Date: Wed, 14 Dec 2022 15:51:31 +0100 Subject: [PATCH 1/2] fix(plugin-lighthouse): fix form factor field in the audit creation form not working with the latest version of lighthouse-audit-service Signed-off-by: Gabrielle Langer --- .changeset/many-mayflies-share.md | 5 +++ plugins/lighthouse/api-report.md | 21 ++++++++++- plugins/lighthouse/src/api.ts | 21 ++++++++++- .../src/components/CreateAudit/index.tsx | 37 ++++++++++++------- 4 files changed, 69 insertions(+), 15 deletions(-) create mode 100644 .changeset/many-mayflies-share.md diff --git a/.changeset/many-mayflies-share.md b/.changeset/many-mayflies-share.md new file mode 100644 index 0000000000..080afc6ac1 --- /dev/null +++ b/.changeset/many-mayflies-share.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-lighthouse': patch +--- + +Fixed "Emulated Form Factor" field in the audit creation form not working with the latest version (1.0.2) of `lighthouse-audit-service`. diff --git a/plugins/lighthouse/api-report.md b/plugins/lighthouse/api-report.md index 17c771e187..e4d945b6dd 100644 --- a/plugins/lighthouse/api-report.md +++ b/plugins/lighthouse/api-report.md @@ -71,6 +71,14 @@ export class FetchError extends Error { get name(): string; } +// @public (undocumented) +export enum FormFactor { + // (undocumented) + Desktop = 'desktop', + // (undocumented) + Mobile = 'mobile', +} + // @public (undocumented) const isLighthouseAvailable: (entity: Entity) => boolean; export { isLighthouseAvailable }; @@ -167,13 +175,24 @@ export class LighthouseRestApi implements LighthouseApi { // @public (undocumented) export const Router: () => JSX.Element; +// @public (undocumented) +export type ScreenEmulation = { + mobile: boolean; + width: number; + height: number; + deviceScaleFactor: number; + disabled: boolean; +} | null; + // @public (undocumented) export interface TriggerAuditPayload { // (undocumented) options: { lighthouseConfig: { settings: { - emulatedFormFactor: string; + formFactor: FormFactor; + screenEmulation: ScreenEmulation; + emulatedFormFactor: FormFactor; }; }; }; diff --git a/plugins/lighthouse/src/api.ts b/plugins/lighthouse/src/api.ts index e4ccefe2a1..0bad997087 100644 --- a/plugins/lighthouse/src/api.ts +++ b/plugins/lighthouse/src/api.ts @@ -85,13 +85,32 @@ export interface Website { /** @public */ export type WebsiteListResponse = LASListResponse; +/** @public */ +export enum FormFactor { + Mobile = 'mobile', + Desktop = 'desktop', +} + +/** @public */ +export type ScreenEmulation = { + mobile: boolean; + width: number; + height: number; + deviceScaleFactor: number; + disabled: boolean; +} | null; + /** @public */ export interface TriggerAuditPayload { url: string; options: { lighthouseConfig: { settings: { - emulatedFormFactor: string; + // For lighthouse 7+ + formFactor: FormFactor; + screenEmulation: ScreenEmulation; + // For lighthouse before 7 + emulatedFormFactor: FormFactor; }; }; }; diff --git a/plugins/lighthouse/src/components/CreateAudit/index.tsx b/plugins/lighthouse/src/components/CreateAudit/index.tsx index f83ad6a8a3..2ee5a675b6 100644 --- a/plugins/lighthouse/src/components/CreateAudit/index.tsx +++ b/plugins/lighthouse/src/components/CreateAudit/index.tsx @@ -25,7 +25,7 @@ import { } from '@material-ui/core'; import React, { useCallback, useState } from 'react'; import { useNavigate } from 'react-router-dom'; -import { lighthouseApiRef } from '../../api'; +import { FormFactor, lighthouseApiRef, ScreenEmulation } from '../../api'; import { useQuery } from '../../utils'; import LighthouseSupportButton from '../SupportButton'; @@ -65,6 +65,20 @@ const useStyles = makeStyles(theme => ({ }, })); +const formFactorToScreenEmulationMap: Record = { + // the default is mobile, so no need to override + mobile: null, + // Values from lighthouse's cli "desktop" preset + // https://github.com/GoogleChrome/lighthouse/blob/a6738e0033e7e5ca308b97c1c36f298b7d399402/lighthouse-core/config/constants.js#L71-L77 + desktop: { + mobile: false, + width: 1350, + height: 940, + deviceScaleFactor: 1, + disabled: false, + }, +}; + export const CreateAuditContent = () => { const errorApi = useApi(errorApiRef); const lighthouseApi = useApi(lighthouseApiRef); @@ -73,7 +87,7 @@ export const CreateAuditContent = () => { const navigate = useNavigate(); const [submitting, setSubmitting] = useState(false); const [url, setUrl] = useState(query.get('url') || ''); - const [emulatedFormFactor, setEmulatedFormFactor] = useState('mobile'); + const [formFactor, setFormFactor] = useState(FormFactor.Mobile); const triggerAudit = useCallback(async (): Promise => { setSubmitting(true); @@ -85,7 +99,9 @@ export const CreateAuditContent = () => { options: { lighthouseConfig: { settings: { - emulatedFormFactor, + formFactor, + emulatedFormFactor: formFactor, + screenEmulation: formFactorToScreenEmulationMap[formFactor], }, }, }, @@ -96,14 +112,7 @@ export const CreateAuditContent = () => { } finally { setSubmitting(false); } - }, [ - url, - emulatedFormFactor, - lighthouseApi, - setSubmitting, - errorApi, - navigate, - ]); + }, [url, formFactor, lighthouseApi, setSubmitting, errorApi, navigate]); return ( <> @@ -146,8 +155,10 @@ export const CreateAuditContent = () => { select required disabled={submitting} - onChange={ev => setEmulatedFormFactor(ev.target.value)} - value={emulatedFormFactor} + onChange={ev => + setFormFactor(ev.target.value as FormFactor) + } + value={formFactor} inputProps={{ 'aria-label': 'Emulated form factor' }} > Mobile From 226099ad031a91deb438ea09b99f53ce9fdd3025 Mon Sep 17 00:00:00 2001 From: Gabrielle Langer Date: Wed, 14 Dec 2022 18:10:04 +0100 Subject: [PATCH 2/2] fix(plugin-lighthouse): incorporate feedback and add unit tests Signed-off-by: Gabrielle Langer --- plugins/lighthouse/api-report.md | 37 ++++----- plugins/lighthouse/src/api.ts | 35 ++++---- .../src/components/CreateAudit/index.test.tsx | 82 +++++++++++++++++++ .../src/components/CreateAudit/index.tsx | 15 +++- 4 files changed, 127 insertions(+), 42 deletions(-) diff --git a/plugins/lighthouse/api-report.md b/plugins/lighthouse/api-report.md index e4d945b6dd..e9089c7e92 100644 --- a/plugins/lighthouse/api-report.md +++ b/plugins/lighthouse/api-report.md @@ -72,12 +72,7 @@ export class FetchError extends Error { } // @public (undocumented) -export enum FormFactor { - // (undocumented) - Desktop = 'desktop', - // (undocumented) - Mobile = 'mobile', -} +export type FormFactor = 'mobile' | 'desktop'; // @public (undocumented) const isLighthouseAvailable: (entity: Entity) => boolean; @@ -140,6 +135,21 @@ export type LighthouseCategoryId = | 'accessibility' | 'best-practices'; +// @public (undocumented) +export type LighthouseConfigSettings = { + formFactor: FormFactor; + screenEmulation: + | { + mobile: boolean; + width: number; + height: number; + deviceScaleFactor: number; + disabled: boolean; + } + | undefined; + emulatedFormFactor: FormFactor; +}; + // @public (undocumented) export const LighthousePage: () => JSX.Element; @@ -175,25 +185,12 @@ export class LighthouseRestApi implements LighthouseApi { // @public (undocumented) export const Router: () => JSX.Element; -// @public (undocumented) -export type ScreenEmulation = { - mobile: boolean; - width: number; - height: number; - deviceScaleFactor: number; - disabled: boolean; -} | null; - // @public (undocumented) export interface TriggerAuditPayload { // (undocumented) options: { lighthouseConfig: { - settings: { - formFactor: FormFactor; - screenEmulation: ScreenEmulation; - emulatedFormFactor: FormFactor; - }; + settings: LighthouseConfigSettings; }; }; // (undocumented) diff --git a/plugins/lighthouse/src/api.ts b/plugins/lighthouse/src/api.ts index 0bad997087..a7197c4b96 100644 --- a/plugins/lighthouse/src/api.ts +++ b/plugins/lighthouse/src/api.ts @@ -86,32 +86,31 @@ export interface Website { export type WebsiteListResponse = LASListResponse; /** @public */ -export enum FormFactor { - Mobile = 'mobile', - Desktop = 'desktop', -} +export type FormFactor = 'mobile' | 'desktop'; /** @public */ -export type ScreenEmulation = { - mobile: boolean; - width: number; - height: number; - deviceScaleFactor: number; - disabled: boolean; -} | null; +export type LighthouseConfigSettings = { + // For lighthouse 7+ + formFactor: FormFactor; + screenEmulation: + | { + mobile: boolean; + width: number; + height: number; + deviceScaleFactor: number; + disabled: boolean; + } + | undefined; + // For lighthouse before 7 + emulatedFormFactor: FormFactor; +}; /** @public */ export interface TriggerAuditPayload { url: string; options: { lighthouseConfig: { - settings: { - // For lighthouse 7+ - formFactor: FormFactor; - screenEmulation: ScreenEmulation; - // For lighthouse before 7 - emulatedFormFactor: FormFactor; - }; + settings: LighthouseConfigSettings; }; }; } diff --git a/plugins/lighthouse/src/components/CreateAudit/index.test.tsx b/plugins/lighthouse/src/components/CreateAudit/index.test.tsx index 59847b516e..60a7bc1301 100644 --- a/plugins/lighthouse/src/components/CreateAudit/index.test.tsx +++ b/plugins/lighthouse/src/components/CreateAudit/index.test.tsx @@ -110,6 +110,88 @@ describe('CreateAudit', () => { }); }); + describe('when creating the audit', () => { + it('sends the correct payload for mobile', async () => { + let triggerAuditPayload: {} | undefined = undefined; + server.use( + rest.post('http://lighthouse/v1/audits', async (req, res, ctx) => { + triggerAuditPayload = await req.json(); + return res(ctx.json(createAuditResponse)); + }), + ); + + const rendered = render( + wrapInTestApp( + + + , + ), + ); + + fireEvent.change(rendered.getByLabelText(/URL/), { + target: { value: 'https://spotify.com' }, + }); + fireEvent.click(rendered.getByText(/Create Audit/)); + + await waitFor(() => + expect(triggerAuditPayload).toMatchObject({ + options: { + lighthouseConfig: { + settings: { formFactor: 'mobile', emulatedFormFactor: 'mobile' }, + }, + }, + url: 'https://spotify.com', + }), + ); + }); + + it('sends the correct payload for desktop', async () => { + let triggerAuditPayload: {} | undefined = undefined; + server.use( + rest.post('http://lighthouse/v1/audits', async (req, res, ctx) => { + triggerAuditPayload = await req.json(); + return res(ctx.json(createAuditResponse)); + }), + ); + + const rendered = render( + wrapInTestApp( + + + , + ), + ); + + fireEvent.change(rendered.getByLabelText(/URL/), { + target: { value: 'https://spotify.com' }, + }); + fireEvent.mouseDown(rendered.getByText(/Mobile/)); + fireEvent.click(rendered.getByText(/Desktop/)); + fireEvent.click(rendered.getByText(/Create Audit/)); + + await waitFor(() => + expect(triggerAuditPayload).toMatchObject({ + options: { + lighthouseConfig: { + settings: { + formFactor: 'desktop', + screenEmulation: { + mobile: false, + width: 1350, + height: 940, + deviceScaleFactor: 1, + disabled: false, + }, + emulatedFormFactor: 'desktop', + }, + }, + }, + url: 'https://spotify.com', + }), + ); + }); + }); + describe('when the audit is successfully created', () => { it('triggers a location change to the table', async () => { useNavigate.mockClear(); diff --git a/plugins/lighthouse/src/components/CreateAudit/index.tsx b/plugins/lighthouse/src/components/CreateAudit/index.tsx index 2ee5a675b6..8ed7dc8996 100644 --- a/plugins/lighthouse/src/components/CreateAudit/index.tsx +++ b/plugins/lighthouse/src/components/CreateAudit/index.tsx @@ -25,7 +25,11 @@ import { } from '@material-ui/core'; import React, { useCallback, useState } from 'react'; import { useNavigate } from 'react-router-dom'; -import { FormFactor, lighthouseApiRef, ScreenEmulation } from '../../api'; +import { + FormFactor, + lighthouseApiRef, + LighthouseConfigSettings, +} from '../../api'; import { useQuery } from '../../utils'; import LighthouseSupportButton from '../SupportButton'; @@ -65,9 +69,12 @@ const useStyles = makeStyles(theme => ({ }, })); -const formFactorToScreenEmulationMap: Record = { +const formFactorToScreenEmulationMap: Record< + FormFactor, + LighthouseConfigSettings['screenEmulation'] +> = { // the default is mobile, so no need to override - mobile: null, + mobile: undefined, // Values from lighthouse's cli "desktop" preset // https://github.com/GoogleChrome/lighthouse/blob/a6738e0033e7e5ca308b97c1c36f298b7d399402/lighthouse-core/config/constants.js#L71-L77 desktop: { @@ -87,7 +94,7 @@ export const CreateAuditContent = () => { const navigate = useNavigate(); const [submitting, setSubmitting] = useState(false); const [url, setUrl] = useState(query.get('url') || ''); - const [formFactor, setFormFactor] = useState(FormFactor.Mobile); + const [formFactor, setFormFactor] = useState('mobile'); const triggerAudit = useCallback(async (): Promise => { setSubmitting(true);