Skip to content

Commit c5e8615

Browse files
authored
Customize rows after import (#2401)
* Add getCustomizeStepRows event * Fix build errors. * Run prettier * Fix integration tests * Remove extra unwrapping. * Make linter happy. * Clear timeout if response was received * Use onConfigUpdate subcription instead of introducing a callback * Fix rebase errors * Fix linter errors * introduce a guard for nextStepDefs[key] * remove unnecessary test * Extend "Given onConfigUpdate behavior" UT with additional state verification * Allow running "Given onConfigUpdate behavior" test on macOS * Add new onboarding test to onboarding.v4.spec.js * Make prettier happy. * refactor duplicated code
1 parent 69239d1 commit c5e8615

8 files changed

Lines changed: 124 additions & 11 deletions

File tree

messaging/lib/messaging.types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ interface Window {
3838
mockResponses: Record<string, import('../index.js').MessageResponse>;
3939
subscriptionEvents: import('../index.js').SubscriptionEvent[];
4040
publishSubscriptionEvent?: (evt: import('../index.js').SubscriptionEvent) => void;
41+
/** Optional map of subscription name -> Set of callbacks (used by onboarding mock transport in tests). */
42+
subscriptions?: Map<string, Set<(data: unknown) => void>>;
4143
mocks: {
4244
outgoing: UnstableMockCall[];
4345
};

special-pages/pages/onboarding/app/global.js

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,29 @@ export function reducer(state, action) {
2323
// console.log('action', action)
2424
// console.groupEnd()
2525

26+
if (action.kind === 'config-update') {
27+
let nextStepDefs = state.stepDefinitions;
28+
if (action.stepDefinitions) {
29+
nextStepDefs = { ...state.stepDefinitions };
30+
for (const [key, value] of Object.entries(action.stepDefinitions)) {
31+
if (typeof value === 'object' && value !== null && nextStepDefs[key]) {
32+
nextStepDefs[key] = { ...nextStepDefs[key], ...value };
33+
} else {
34+
nextStepDefs[key] = value;
35+
}
36+
}
37+
}
38+
let nextOrder = state.order;
39+
if (action.exclude) {
40+
nextOrder = state.order.filter((id) => !action.exclude?.includes(id));
41+
}
42+
return {
43+
...state,
44+
stepDefinitions: nextStepDefs,
45+
order: nextOrder,
46+
step: nextStepDefs[state.activeStep] ?? state.step,
47+
};
48+
}
2649
switch (state.status.kind) {
2750
case 'idle': {
2851
switch (action.kind) {
@@ -198,7 +221,7 @@ export function GlobalProvider({ order, children, stepDefinitions, messaging, fi
198221
dispatch(msg);
199222

200223
/**
201-
* Side effects that don't impact global state
224+
* Side effects that don't impact global state (advance to non-customize step, or other message kinds).
202225
*/
203226
if (msg.kind === 'advance') {
204227
messaging.stepCompleted({ id: state.activeStep });
@@ -213,6 +236,16 @@ export function GlobalProvider({ order, children, stepDefinitions, messaging, fi
213236
[state, messaging],
214237
);
215238

239+
useEffect(() => {
240+
const unsubscribe = messaging.onConfigUpdate((data) => {
241+
dispatch({
242+
kind: 'config-update',
243+
stepDefinitions: data.stepDefinitions,
244+
exclude: /** @type {import('./types.js').ConfigUpdateEvent['exclude']} */ (data.exclude),
245+
});
246+
});
247+
return unsubscribe;
248+
}, [messaging]);
216249
// handle *fatal* state (from error boundary)
217250
useEffect(() => {
218251
if (state.status.kind !== 'fatal') return;

special-pages/pages/onboarding/app/messages.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ export class OnboardingMessages {
159159
this.messaging.notify('reportInitException', params);
160160
}
161161

162+
/**
163+
* Subscribe to config updates pushed by native (e.g. customize step rows).
164+
* @param {(data: {stepDefinitions?: Record<string, any>, exclude?: string[]}) => void} params
165+
* @returns {() => void}
166+
*/
167+
onConfigUpdate(params) {
168+
return this.messaging.subscribe('onConfigUpdate', params);
169+
}
170+
162171
/**
163172
* Sent when the user wants to enable or disable ad blocking.
164173
*

special-pages/pages/onboarding/app/types.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ export const ORDER_V4 = ['welcome', 'getStarted', 'makeDefaultSingle', 'systemSe
9898
* | DismisstoSettingsEvent
9999
* | ErrorBoundaryEvent
100100
* | ShowOverlayEvent
101-
* | DismissOverlayEvent} GlobalEvents
101+
* | DismissOverlayEvent
102+
* | ConfigUpdateEvent} GlobalEvents
102103
* All the events that the UI can dispatch
103104
* @typedef {{ kind: "enqueue-next"; }} NextEvent
104105
* @typedef {{ kind: "advance" }} AdvanceEvent
@@ -111,7 +112,7 @@ export const ORDER_V4 = ['welcome', 'getStarted', 'makeDefaultSingle', 'systemSe
111112
* @typedef {{ kind: "title-complete"; }} TitleCompleteEvent
112113
* @typedef {{ kind: "show-overlay"; overlay: OverlayId }} ShowOverlayEvent
113114
* @typedef {{ kind: "dismiss-overlay" }} DismissOverlayEvent
114-
*
115+
* @typedef {{ kind: "config-update"; stepDefinitions?: Record<string, any>; exclude?: Step['id'][] }} ConfigUpdateEvent
115116
*/
116117

117118
/** @type {ImportMeta['injectName'][]} */

special-pages/pages/onboarding/integration-tests/onboarding.page.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,16 @@ export class OnboardingPage {
4848
}
4949

5050
withInitData(data) {
51-
this.mocks.defaultResponses({
52-
...this.defaultResponses,
53-
init: data,
54-
});
51+
this.defaultResponses = { ...this.defaultResponses, init: data };
52+
this.mocks.defaultResponses(this.defaultResponses);
53+
}
54+
55+
/**
56+
* Push an onConfigUpdate subscription event (e.g. stepDefinitions.customize.rows for integration tests).
57+
* @param {{ stepDefinitions?: Record<string, any>; exclude?: string[] }} payload
58+
*/
59+
async pushConfigUpdate(payload) {
60+
await this.mocks.simulateSubscriptionMessage('onConfigUpdate', payload);
5561
}
5662

5763
/**

special-pages/pages/onboarding/integration-tests/onboarding.v3.spec.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,5 +765,28 @@ test.describe('onboarding v3', () => {
765765
// ▶️ Then I can toggle it afterward
766766
await onboarding.startBrowsing();
767767
});
768+
769+
test.describe('Given onConfigUpdate behavior', () => {
770+
test('When config update has reduced customize rows (no bookmarks), only those rows are shown', async ({
771+
page,
772+
}, workerInfo) => {
773+
const onboarding = OnboardingV3Page.create(page, workerInfo);
774+
onboarding.withInitData({
775+
order: 'v3',
776+
stepDefinitions: { systemSettings: { rows: ['dock', 'import', 'default-browser'] } },
777+
});
778+
await onboarding.reducedMotion();
779+
await onboarding.openPage({ env: 'app', page: 'customize' });
780+
// before push — default rows include bookmarks
781+
await page.getByRole('button', { name: 'Show Bookmarks Bar' }).waitFor({ timeout: 10000 });
782+
await expect(page.getByRole('button', { name: 'Show Bookmarks Bar' })).toBeVisible();
783+
// push config update removing bookmarks
784+
await onboarding.pushConfigUpdate({ stepDefinitions: { customize: { rows: ['session-restore', 'home-shortcut'] } } });
785+
// after push — bookmarks gone
786+
await page.getByRole('button', { name: 'Enable Session Restore' }).waitFor({ timeout: 10000 });
787+
await expect(page.getByRole('button', { name: 'Show Bookmarks Bar' })).not.toBeVisible();
788+
await expect(page.getByRole('button', { name: 'Enable Session Restore' })).toBeVisible();
789+
});
790+
});
768791
});
769792
});

special-pages/pages/onboarding/integration-tests/onboarding.v4.spec.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,5 +462,28 @@ test.describe('onboarding v4', () => {
462462
// ▶️ Then I can toggle it afterward
463463
await onboarding.startBrowsing();
464464
});
465+
466+
test.describe('Given onConfigUpdate behavior', () => {
467+
test('When config update has reduced customize rows (no bookmarks), only those rows are shown', async ({
468+
page,
469+
}, workerInfo) => {
470+
const onboarding = OnboardingV4Page.create(page, workerInfo);
471+
onboarding.withInitData({
472+
order: 'v4',
473+
stepDefinitions: { systemSettings: { rows: ['dock', 'import', 'default-browser'] } },
474+
});
475+
await onboarding.reducedMotion();
476+
await onboarding.openPage({ env: 'app', page: 'customize' });
477+
// before push - default rows include bookmarks
478+
await page.getByRole('button', { name: 'Show Bookmarks Bar' }).waitFor({ timeout: 10000 });
479+
await expect(page.getByRole('button', { name: 'Show Bookmarks Bar' })).toBeVisible();
480+
// push config update removing bookmarks
481+
await onboarding.pushConfigUpdate({ stepDefinitions: { customize: { rows: ['session-restore', 'home-shortcut'] } } });
482+
// after push - bookmarks gone
483+
await page.getByRole('button', { name: 'Enable Session Restore' }).waitFor({ timeout: 10000 });
484+
await expect(page.getByRole('button', { name: 'Show Bookmarks Bar' })).not.toBeVisible();
485+
await expect(page.getByRole('button', { name: 'Enable Session Restore' })).toBeVisible();
486+
});
487+
});
465488
});
466489
});

special-pages/pages/onboarding/src/mock-transport.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ import { TestTransportConfig } from '@duckduckgo/messaging';
77
const url = new URL(window.location.href);
88

99
export function mockTransport() {
10+
if (typeof window !== 'undefined' && window.__playwright_01) {
11+
window.__playwright_01.publishSubscriptionEvent = (/** @type {{ subscriptionName: string; params?: unknown }} */ evt) => {
12+
window.__playwright_01?.subscriptions
13+
?.get(evt.subscriptionName)
14+
?.forEach((/** @type {(data: unknown) => void} */ cb) => cb(evt.params));
15+
};
16+
}
1017
return new TestTransportConfig({
1118
notify(_msg) {
1219
window.__playwright_01?.mocks?.outgoing?.push?.({ payload: structuredClone(_msg) });
@@ -79,11 +86,20 @@ export function mockTransport() {
7986
},
8087
subscribe(_msg, callback) {
8188
window.__playwright_01?.mocks?.outgoing?.push?.({ payload: structuredClone(_msg) });
82-
83-
callback(null);
84-
89+
if (!window.__playwright_01) {
90+
return () => {};
91+
}
92+
const msg = /** @type {{ method?: string; subscriptionName?: string }} */ (_msg);
93+
const name = typeof _msg === 'string' ? _msg : (msg.subscriptionName ?? msg.method ?? 'onConfigUpdate');
94+
if (!window.__playwright_01.subscriptions) {
95+
window.__playwright_01.subscriptions = new Map();
96+
}
97+
if (!window.__playwright_01.subscriptions.has(name)) {
98+
window.__playwright_01.subscriptions.set(name, new Set());
99+
}
100+
window.__playwright_01.subscriptions.get(name)?.add(callback);
85101
return () => {
86-
// any cleanup
102+
window.__playwright_01?.subscriptions?.get(name)?.delete(callback);
87103
};
88104
},
89105
});

0 commit comments

Comments
 (0)