NPS v2: Add the skeleton of the marketing package, including the empty NPS component #95046

southp · 2024-10-01T06:12:36Z

Related to #94874

Proposed Changes

This PR establishes the foundational package structure of the marketing package with NPS v2 as the first component within it. Following the discussions in #94874, this PR attempts the direction of an isolated package with all the NPS v2 related components and data-related hooks consolidated inside.

The package itself is initiated by the following way:
- The package outline was generated by yarn run generate
- Added the placeholder CHANGELOG.md
- Adjusted several dependencies in the generated template as the peer dependencies since those should be managed by the parent project in my opinion.
- Added calypso-build as a peer dependency so copy-assets is available.
NPS v2 resides in src/nps.
The hooks are separated into queries and mutations. The unit tests are located in the respected __tests__ directories, following the current jest.config setting.
Added the basic configuration of Storybook. There are currently two placeholder stories: desktop and mobile, since I expect I'd need to iterate on the both layout the most.

Why are these changes being made?

It's the foundation of the NPS v2 project.

Testing Instructions

The most important factor is to verify if the package structure makes sense.
yarn run test-packages -- marketing should run the newly added two tests here and they should pass.
yarn run @automattic/marketing storybook should run the storybook instance. Note that the NPS component is meant to be rough for now:

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

component.

github-actions · 2024-10-01T06:12:49Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-121360

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-121360&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-121360&env=a8c-for-agencies

matticbot · 2024-10-01T06:17:50Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

marketing package.

chriskmnds

Looks great! Left some comments, but not sure if anything critical.

chriskmnds · 2024-10-08T08:58:10Z

packages/marketing/package.json

+{
+	"name": "@automattic/marketing",
+	"version": "1.0.0",
+	"description": "The package where the marketing components related to Automattic products are located.",


I like this, overall. It means we can add stuff from JP/eslsewhere here too.

However, if that's not the intention - then it could also be @automattic/wpcom-marketting for consideration.

chriskmnds · 2024-10-08T09:05:41Z

packages/marketing/src/nps/queries/use-check-nps-survey-eligibility.ts

Have we considered adding a nps slice to @automattic/data-stores? It has pros & cons. I think it's tempting as a model of separation, so we keep all data-related stuff under there. It might be easier that way to focus on updates, keeping things consistent with other related data hooks, etc. But I can see how living here makes sense.

chriskmnds · 2024-10-08T09:12:36Z

packages/marketing/src/nps/index.tsx

+					On a scale from 0–10, how likely are you to recommend WordPress.com to a friend or
+					colleague?
+				</legend>
+				<div className="nps-scale">


For classnames we should probably follow some convention to avoid clashes. Something like [name of package]-[name of component]__[...]. So for this, maybe marketting-nps__scale?

Which brings up the question whether marketting might be too generic (or not)

chriskmnds · 2024-10-08T09:13:16Z

packages/marketing/src/nps/index.tsx

+					colleague?
+				</legend>
+				<div className="nps-scale">
+					<span className="label-left">Not likely</span>


Similarly. Could be marketting-nps__scale-label-left, etc.

chriskmnds · 2024-10-08T09:18:33Z

packages/marketing/src/nps/index.tsx

+	);
+}
+
+export function Nps() {


Once we get a fully working version, we can keep this as the plain/view only component (that we can also build Stories for), and ship the wrapper as Nps that would also include the handlers for showing/not showing, mutating, etc.

chriskmnds · 2024-10-08T09:23:30Z

packages/marketing/src/nps/mutations/use-submit-nps-survey-mutation.ts

+import { useMutation } from '@tanstack/react-query';
+import wpcomRequest from 'wpcom-proxy-request';


We are missing these dependencies in package.json 🤔

southp · 2024-10-14T07:17:31Z

This PR is now paused for further assessment of the overall project: p5uIfZ-g9B-p2#comment-24070 .

southp · 2024-10-25T01:45:18Z

Closing it since there isn't any sign that the work will be picked up again in the near future.

Add the skeleton of the marketing package, with nps as the 1st

3a4dd0e

component.

southp added [Status] In Progress NPS v2 Tasks related to the Net Promoter Score v2 project labels Oct 1, 2024

southp self-assigned this Oct 1, 2024

southp mentioned this pull request Oct 1, 2024

NPS v2: Establish the foundational package. #94874

Open

2 tasks

southp added 8 commits October 2, 2024 11:23

Implement the foundational structure of Storybook setup for the

e19a532

marketing package.

Draft all the sections & components in using the standard HTML elements.

75f6bb7

Separate and extend the stories into "Desktop" & "Mobile"

4a25795

Add an empty submit handler

dd219e6

Scaffold the query hooks

0958374

Scaffold the query hooks, starting from the eligibility check.

6d6113c

Scaffold the mutation hooks

1e4e108

Add the NPS survey submission related types

c735aad

southp marked this pull request as ready for review October 8, 2024 06:46

southp added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Oct 8, 2024

southp requested review from a team, jeyip and chriskmnds October 8, 2024 06:50

chriskmnds reviewed Oct 8, 2024

View reviewed changes

southp closed this Oct 25, 2024

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NPS v2: Add the skeleton of the marketing package, including the empty NPS component #95046

NPS v2: Add the skeleton of the marketing package, including the empty NPS component #95046

southp commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited by matticbot

Loading

matticbot commented Oct 1, 2024

chriskmnds left a comment

chriskmnds Oct 8, 2024

chriskmnds Oct 8, 2024

chriskmnds Oct 8, 2024

chriskmnds Oct 8, 2024

chriskmnds Oct 8, 2024

chriskmnds Oct 8, 2024

southp commented Oct 14, 2024

southp commented Oct 25, 2024

		import { useMutation } from '@tanstack/react-query';
		import wpcomRequest from 'wpcom-proxy-request';

NPS v2: Add the skeleton of the marketing package, including the empty NPS component #95046

NPS v2: Add the skeleton of the marketing package, including the empty NPS component #95046

Conversation

southp commented Oct 1, 2024 • edited Loading

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

github-actions bot commented Oct 1, 2024 • edited by matticbot Loading

matticbot commented Oct 1, 2024

chriskmnds left a comment

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

chriskmnds Oct 8, 2024

Choose a reason for hiding this comment

southp commented Oct 14, 2024

southp commented Oct 25, 2024

southp commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited by matticbot

Loading