diff --git a/plugins/newtab/skills/widgets-scaffold/SKILL.md b/plugins/newtab/skills/widgets-scaffold/SKILL.md index 9e00233..4baa324 100644 --- a/plugins/newtab/skills/widgets-scaffold/SKILL.md +++ b/plugins/newtab/skills/widgets-scaffold/SKILL.md @@ -10,10 +10,12 @@ The **Widget Registry** (`common/WidgetsRegistry.mjs`) is the single source of truth for every widget. Adding a widget is now mostly declarative: you add one registry entry plus one component-registry entry, and shared helpers (`isWidgetEnabled`, `resolveWidgetSize`, `resolveWidgetHasSidebar`, -`getHideAllTargets`, `resolveWidgetOrder`) and shared UI (`useSizeSubmenu`, -`MoveSubmenu`, `WidgetWrapper`) do the wiring that used to be hand-copied into -`Widgets.jsx` and `Base.jsx`. Do **not** hand-wire enabled-logic, size -derivation, hide-all, or the null-guard into those files anymore. +`getHideAllTargets`, `resolveWidgetOrder`), shared UI (`WidgetMenuFooter`, +`SizeSubmenu`, `MoveSubmenu`, `WidgetWrapper`), and the shared +`useWidgetTelemetry` hook do the wiring that used to be hand-copied into +`Widgets.jsx`, `Base.jsx`, and every widget's menu and telemetry. Do **not** +hand-wire enabled-logic, size derivation, hide-all, the null-guard, the menu +footer, or telemetry payloads anymore. ## Naming convention (read first) @@ -34,9 +36,14 @@ Widget telemetry is fully generic and already defined once for all widgets in `telemetryName`) plus `widget_size`, `error_type`, `user_action`, etc. A new widget therefore adds **nothing** to `metrics.yaml` and requires **no** -`./mach newtab channel-metrics-diff` run. It just emits the shared events with -its own `telemetryName` as `widget_name` (impression via the intersection -observer, interactions via `WIDGETS_USER_EVENT`, errors via `widgets_error`). +`./mach newtab channel-metrics-diff` run. It emits the shared events through the +`useWidgetTelemetry({ dispatch, widget, widgetSize })` hook, which returns +`impressionRef` (attach to the widget root — fires `widgets_impression` once via +its own IntersectionObserver), `recordUserAction(action, { source, value })` +(`widgets_user_event`), `recordEnabled` (`widgets_enabled`), `recordImpression` +(manual one-shot), and `recordError` (`widgets_error`). Do NOT hand-build these +payloads or wire up your own observer — the hook keys every event on the +registry entry's `telemetryName` for you. Only edit `metrics.yaml` if the widget needs a genuinely new event shape that the shared `widgets_*` events cannot express — which is rare. Never scaffold a per-widget `widgets.{key}.*` metric. @@ -81,10 +88,25 @@ The registry-centric core (do these first, in order): pref directly. - Accept the standard props: `dispatch`, `handleUserInteraction` (interactive widgets only), `isMaximized`, `widgetsMayBeMaximized`, `widgetEnabledMap`. - - Resize submenu via the `useSizeSubmenu(handleChangeSize)` hook (do NOT - hand-roll the click listener). Gate the submenu on - `widgetsMayBeMaximized`. Include `"small"` only if `validSizes` has it. - - Reorder via ``. + - Emit telemetry through the `useWidgetTelemetry({ dispatch, widget: ENTRY, + widgetSize })` hook: attach the returned `impressionRef` to the widget's + root element and call `recordUserAction(action, { source, value })` for + interactions (`value` sets `action_value`; pass `size` to override the + reported `widget_size`, as the resize handler does). Do **not** hand-build + `WIDGETS_IMPRESSION` / `WIDGETS_USER_EVENT` payloads or wire up your own + `IntersectionObserver`. + - Render the shared `` as the last child of the menu's + ``; it owns the common trailing block in a fixed order + (divider, Change size, Move, Hide widget, Learn more — Learn more opens in + a new tab). Put widget-specific items **above** it. Pass the Change size + submenu via the `sizeSubmenu` prop as ``, + gated on `widgetsMayBeMaximized` (pass `null` otherwise); include `"small"` + only if `validSizes` has it. Do **not** hand-roll the size submenu, + `MoveSubmenu`, the Hide item, or the Learn more item — that hand-rolled + footer is exactly what drifted out of sync (Bug 2046045 / D306294). The + footer's `onLearnMore` is only for the extra `learn_more` telemetry; it + dispatches `OPEN_LINK` (with `where: "tab"`) itself. - Render its own `
` root. Do **not** add a per-widget Nova gate — the container (`Widgets.jsx`) renders `WIDGET_ROW_COMPONENTS` only in its Nova branch, so the widget already diff --git a/plugins/newtab/skills/widgets-scaffold/references/ExampleWidget/ExampleWidget.jsx b/plugins/newtab/skills/widgets-scaffold/references/ExampleWidget/ExampleWidget.jsx index 96efa05..8f730b7 100644 --- a/plugins/newtab/skills/widgets-scaffold/references/ExampleWidget/ExampleWidget.jsx +++ b/plugins/newtab/skills/widgets-scaffold/references/ExampleWidget/ExampleWidget.jsx @@ -3,17 +3,19 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ // eslint-disable-next-line no-unused-vars -import React, { useCallback, useRef } from "react"; +import React, { useCallback } from "react"; import { useSelector, batch } from "react-redux"; import { actionCreators as ac, actionTypes as at } from "common/Actions.mjs"; -import { useIntersectionObserver, useSizeSubmenu } from "../../../lib/utils"; import { WIDGET_REGISTRY, resolveWidgetSize } from "common/WidgetsRegistry.mjs"; -import { MoveSubmenu } from "../MoveSubmenu"; +import { SizeSubmenu } from "../SizeSubmenu"; +import { WidgetMenuFooter } from "../WidgetMenuFooter"; +import { useWidgetTelemetry } from "../useWidgetTelemetry"; // Only present for interactive widgets. Remove entirely for view-only widgets. const USER_ACTION_TYPES = { CHANGE_SIZE: "change_size", DO_THING: "do_thing", + LEARN_MORE: "learn_more", }; // Constants for any extra widget-specific prefs read inside this component. @@ -42,26 +44,20 @@ function ExampleWidget({ // Size comes from the registry helper: user-set pref > trainhop suggestion // > registry defaultSize. Never read the size pref directly. const widgetSize = resolveWidgetSize(EXAMPLE_ENTRY, prefs); - const impressionFired = useRef(false); - - const handleIntersection = useCallback(() => { - if (impressionFired.current) { - return; - } - impressionFired.current = true; - dispatch( - ac.AlsoToMain({ - type: at.WIDGETS_IMPRESSION, - data: { - // telemetryName from the registry entry (snake_case, no "widget"). - widget_name: "example", - widget_size: widgetSize, - }, - }) - ); - }, [dispatch, widgetSize]); - - const widgetRef = useIntersectionObserver(handleIntersection); + + // Shared telemetry hook. Do NOT hand-build WIDGETS_IMPRESSION / + // WIDGETS_USER_EVENT payloads or wire up your own IntersectionObserver: + // attach `impressionRef` to the widget root for the one-shot impression, and + // call `recordUserAction(action, { source, value })` for interactions + // (`value` sets `action_value`; pass `size` to override the reported + // `widget_size`). `widget` is the registry entry; its `telemetryName` + // becomes `widget_name`. The hook also returns `recordImpression`, + // `recordEnabled`, and `recordError`. + const { impressionRef, recordUserAction } = useWidgetTelemetry({ + dispatch, + widget: EXAMPLE_ENTRY, + widgetSize, + }); // Call handleUserInteraction("") after any interaction that should mark // the widget as "interacted with" for the feature-highlight flow. @@ -72,45 +68,12 @@ function ExampleWidget({ function handleDoThing() { batch(() => { - // Your main action dispatch goes here, e.g. ac.AlsoToMain(...) - - dispatch( - ac.OnlyToMain({ - type: at.WIDGETS_USER_EVENT, - data: { - widget_name: "example", - widget_source: "widget", - user_action: USER_ACTION_TYPES.DO_THING, - widget_size: widgetSize, - }, - }) - ); + // Your main action dispatch goes here, e.g. dispatch(ac.AlsoToMain(...)) + recordUserAction(USER_ACTION_TYPES.DO_THING, { source: "widget" }); }); handleInteraction(); } - function handleExampleHide() { - batch(() => { - dispatch( - ac.OnlyToMain({ - type: at.SET_PREF, - data: { name: EXAMPLE_ENTRY.enabledPref, value: false }, - }) - ); - dispatch( - ac.OnlyToMain({ - type: at.WIDGETS_ENABLED, - data: { - widget_name: "example", - widget_source: "context_menu", - enabled: false, - widget_size: widgetSize, - }, - }) - ); - }); - } - const handleChangeSize = useCallback( size => { batch(() => { @@ -120,49 +83,24 @@ function ExampleWidget({ data: { name: EXAMPLE_ENTRY.sizePref, value: size }, }) ); - dispatch( - ac.OnlyToMain({ - type: at.WIDGETS_USER_EVENT, - data: { - widget_name: "example", - widget_source: "context_menu", - user_action: USER_ACTION_TYPES.CHANGE_SIZE, - action_value: size, - widget_size: size, - }, - }) - ); + // `value` is action_value; `size` overrides the reported widget_size. + // For a resize they're the new size, but they are distinct fields — + // without `size` the event would report the pre-change widget_size. + recordUserAction(USER_ACTION_TYPES.CHANGE_SIZE, { + source: "context_menu", + value: size, + size, + }); }); }, - [dispatch] + [dispatch, recordUserAction] ); - // Shared hook: returns a ref to attach to the resize submenu's . - // It listens at the panel-list root and walks composedPath() to find the - // clicked size by its data-size attribute. Do NOT hand-roll this — React's - // synthetic onClick does not cross the panel-list shadow-DOM boundary. - const sizeSubmenuRef = useSizeSubmenu(handleChangeSize); - + // Hide and the "Learn more" link (opened in a new tab) are owned by + // WidgetMenuFooter. This callback only records the extra learn_more event the + // footer fires through `onLearnMore` — do not dispatch OPEN_LINK yourself. function handleLearnMore() { - batch(() => { - dispatch( - ac.OnlyToMain({ - type: at.OPEN_LINK, - data: { url: "https://support.mozilla.org/kb/firefox-new-tab-widgets" }, - }) - ); - dispatch( - ac.OnlyToMain({ - type: at.WIDGETS_USER_EVENT, - data: { - widget_name: "example", - widget_source: "context_menu", - user_action: "learn_more", - widget_size: widgetSize, - }, - }) - ); - }); + recordUserAction(USER_ACTION_TYPES.LEARN_MORE, { source: "context_menu" }); } // No per-widget Nova gate. The widgets container (Widgets.jsx) already renders @@ -174,9 +112,7 @@ function ExampleWidget({ return (
{ - widgetRef.current = [el]; - }} + ref={impressionRef} >

Example Widget

@@ -188,41 +124,34 @@ function ExampleWidget({ type="ghost" /> - {/* Additional context menu items from the spec go here, first. */} - - {/* Resize submenu — only when sizing is available in this layout. - Include "small" in the map only when validSizes contains it. */} - {widgetsMayBeMaximized && ( - - - - {["medium", "large"].map(size => ( - - ))} - - - )} - - {/* Reorder submenu — shared component, reads widgets.order. */} - - - - + ) : null + } />
diff --git a/plugins/newtab/skills/widgets-scaffold/references/notes.md b/plugins/newtab/skills/widgets-scaffold/references/notes.md index 031c2cd..1dc3b41 100644 --- a/plugins/newtab/skills/widgets-scaffold/references/notes.md +++ b/plugins/newtab/skills/widgets-scaffold/references/notes.md @@ -204,39 +204,67 @@ with no messages after it is a lint error (GC04). Never invent user-facing string values — copy comes from a copywriter. Wait for the literal text before adding the message. -## Widget resize context menu — use the useSizeSubmenu hook +## Context menu footer — use WidgetMenuFooter + SizeSubmenu -The resize submenu is a `` containing a `` of -size `` rows. Wire its clicks with the shared -`useSizeSubmenu(handleChangeSize)` hook from `content-src/lib/utils`: +The trailing block of every widget menu is shared. Render `` as +the last child of the menu's ``, with widget-specific items above it. +It renders, in a fixed order: a divider, Change size, Move, Hide widget, and Learn +more (which it opens in a **new tab**). ```jsx -const sizeSubmenuRef = useSizeSubmenu(handleChangeSize); +import { SizeSubmenu } from "../SizeSubmenu"; +import { WidgetMenuFooter } from "../WidgetMenuFooter"; // ... - + + {/* widget-specific items go here, ABOVE the footer */} + + ) : null + } + /> + ``` -Do NOT hand-roll a `useEffect` + `composedPath()` click listener — that's exactly -what the hook now encapsulates. React's synthetic `onClick` does not cross the -`panel-list` shadow-DOM boundary, which is why the hook listens at the root. +`SizeSubmenu` owns the resize submenu — it uses the `useSizeSubmenu` hook +internally, so you no longer wire `sizeSubmenuRef` yourself, and you must NOT +hand-roll a `useEffect` + `composedPath()` listener (React's synthetic `onClick` +does not cross the `panel-list` shadow-DOM boundary). Gate it on +`widgetsMayBeMaximized` (pass `null` otherwise), and build `sizes` from the +entry's `validSizes` — include `"small"` only when present. -Gate the submenu on `widgetsMayBeMaximized`. Build the size list from the entry's -`validSizes` — include `"small"` only when it is present. +`WidgetMenuFooter` renders `MoveSubmenu` for you (reads/writes `widgets.order` +via `resolveWidgetOrder`, renders nothing when the widget can't move), dispatches +the Hide pref + `widgets_enabled` telemetry, and opens the Learn more link with +`where: "tab"`. `onLearnMore` is optional and fires only the extra `learn_more` +`widgets_user_event` — do NOT dispatch `OPEN_LINK` yourself. -## Widget reorder — the MoveSubmenu component +This footer used to be copy-pasted into every widget and drifted out of sync +(inconsistent order, a missing divider, Learn more opening in the current tab); +Bug 2046045 / D306294 consolidated it. Keep new widgets on the shared components. -Widgets can be reordered. Render the shared component in the context menu, before -Hide/Learn more: +## Telemetry — use the useWidgetTelemetry hook -```jsx -import { MoveSubmenu } from "../MoveSubmenu"; -// ... - -``` - -It reads/writes the `widgets.order` pref via `resolveWidgetOrder`, renders nothing -when the widget can't move, and (like the size submenu) listens at the panel-list -root for `data-move-dir` clicks. You don't write any order logic yourself. +Do not hand-build `WIDGETS_IMPRESSION` / `WIDGETS_USER_EVENT` payloads or wire an +`IntersectionObserver`. Call `useWidgetTelemetry({ dispatch, widget: ENTRY, +widgetSize })`, attach the returned `impressionRef` to the widget's root element, +and use `recordUserAction(action, { source, value })` for interactions. It also +returns `recordEnabled`, `recordImpression`, and `recordError`, and keys every +event on the entry's `telemetryName` for you. ## Standard widget props @@ -292,8 +320,10 @@ change with a telemetry dispatch, etc. ## Learn More URL -Always point to `https://support.mozilla.org/kb/firefox-new-tab-widgets`. Do not -create a widget-specific SUMO page — all widgets share this URL. +All widgets share `https://support.mozilla.org/kb/firefox-new-tab-widgets`, and +`WidgetMenuFooter` already opens it (in a new tab) for you — do not dispatch +`OPEN_LINK` or hardcode the URL in your widget. Do not create a widget-specific +SUMO page. ## Use the `.jsx` extension for content-src files