Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions packages/marketing/.storybook/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require( '@automattic/calypso-storybook' )();
3 changes: 3 additions & 0 deletions packages/marketing/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 1.0.0

The initial version.
3 changes: 3 additions & 0 deletions packages/marketing/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Marketing

The package where the marketing components related to automattic products are located
6 changes: 6 additions & 0 deletions packages/marketing/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
preset: '../../test/packages/jest-preset.js',
testEnvironment: 'jsdom',
testMatch: [ '<rootDir>/**/__tests__/**/*.[jt]s?(x)', '!**/.eslintrc.*' ],
transformIgnorePatterns: [ 'node_modules/(?!gridicons)(?!.*\\.svg)' ],
};
47 changes: 47 additions & 0 deletions packages/marketing/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"name": "@automattic/marketing",
"version": "1.0.0",
"description": "The package where the marketing components related to Automattic products are located.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"homepage": "https://github.com/Automattic/wp-calypso",
"license": "GPL-2.0-or-later",
"author": "Automattic Inc.",
"main": "dist/cjs/index.js",
"module": "dist/esm/index.js",
"calypso:src": "src/index.tsx",
"sideEffects": [
"*.css",
"*.scss"
],
"repository": {
"type": "git",
"url": "git+https://github.com/Automattic/wp-calypso.git",
"directory": "packages/marketing"
},
"publishConfig": {
"access": "public"
},
"bugs": "https://github.com/Automattic/wp-calypso/issues",
"types": "dist/types",
"scripts": {
"clean": "tsc --build ./tsconfig.json ./tsconfig-cjs.json --clean && rm -rf dist",
"build": "tsc --build ./tsconfig.json ./tsconfig-cjs.json && copy-assets",
"prepack": "yarn run clean && yarn run build",
"watch": "tsc --build ./tsconfig.json --watch",
"storybook": "sb dev"
},
"peerDependencies": {
"postcss": "^8.4.5",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"tslib": "^2.3.0"
},
"devDependencies": {
"@automattic/calypso-build": "workspace:^",
"@automattic/calypso-storybook": "workspace:^",
"@automattic/calypso-typescript-config": "workspace:^",
"@storybook/cli": "^7.6.19",
"@storybook/react": "^7.6.19",
"typescript": "^4.7.4",
"webpack": "^5.94.0"
}
}
1 change: 1 addition & 0 deletions packages/marketing/src/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as Nps } from './nps';
3 changes: 3 additions & 0 deletions packages/marketing/src/nps/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Nps

The NPS(Net Promoter Score) survey form.
22 changes: 22 additions & 0 deletions packages/marketing/src/nps/index.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Nps from '.';
import type { Meta, StoryObj } from '@storybook/react';

type NpsStory = StoryObj< typeof Nps >;

const meta: Meta< typeof Nps > = {
title: 'Nps v2',
component: Nps,
};

export default meta;

export const Desktop: NpsStory = {
args: {},
};

export const Mobile: NpsStory = {
args: {},
parameters: {
viewport: { defaultViewport: 'mobile1' },
},
};
38 changes: 38 additions & 0 deletions packages/marketing/src/nps/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { FormEvent } from 'react';

function NpsScore( score: number ) {
return (
<label>
<input type="radio" name="nps-score" value={ score } />
{ score }
</label>
);
}

export function Nps() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const scores = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];

const handleSubmit = ( event: FormEvent< HTMLFormElement > ) => {
event.preventDefault();
};

return (
<form onSubmit={ handleSubmit }>
<h1>How are we doing so far?</h1>
<fieldset>
<legend>
On a scale from 0–10, how likely are you to recommend WordPress.com to a friend or
colleague?
</legend>
<div className="nps-scale">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

<span className="label-left">Not likely</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

<div className="score-buttons">{ scores.map( ( score ) => NpsScore( score ) ) }</div>
<span className="label-right">Definitely</span>
</div>
</fieldset>
<button type="submit">Submit</button>
</form>
);
}

export default Nps;
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { waitFor } from '@testing-library/dom';
import { renderHook } from '@testing-library/react';
import React from 'react';
import wpcomRequest from 'wpcom-proxy-request';
import { SubmitNpsSurveyResponse } from '../../types';
import useSubmitNpsSurveyMutation from '../use-submit-nps-survey-mutation';

jest.mock( 'wpcom-proxy-request', () => ( {
__esModule: true,
default: jest.fn(),
} ) );

const queryClient = new QueryClient();
const wrapper = ( { children } ) => (
<QueryClientProvider client={ queryClient }>{ children }</QueryClientProvider>
);

// TODO
// Cover the failing cases
describe( 'useSubmitNpsSurveyMutation', () => {
beforeEach( () => {
jest.clearAllMocks();
} );

test( 'successfully submit the score', async () => {
( wpcomRequest as jest.Mock ).mockImplementation( (): Promise< SubmitNpsSurveyResponse > => {
return Promise.resolve( {
result: true,
} );
} );

const { result } = renderHook( () => useSubmitNpsSurveyMutation( 'test-survey' ), { wrapper } );

result.current.mutate( { score: 10, feedback: 'profit!' } );

await waitFor( () => {
expect( result.current.isSuccess ).toBe( true );
expect( result.current.data ).toEqual( {
result: true,
} );
} );
} );
} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { useMutation } from '@tanstack/react-query';
import wpcomRequest from 'wpcom-proxy-request';
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are missing these dependencies in package.json 🤔

import { SubmitNpsSurveyParams, SubmitNpsSurveyResponse } from '../types';

// TBD
// Note that the current endpoint is designed to be able to take score/dismissed/feedback separately by one endpoint. We can consider to separate the endpoint, separate the hook, or separate the both later. Currently it justadheres the existing design to begin with.
function useSubmitNpsSurveyMutation( surveyName: string ) {
return useMutation( {
mutationFn: async ( { score, dismissed, feedback }: SubmitNpsSurveyParams ) => {
const response = await wpcomRequest< SubmitNpsSurveyResponse >( {
path: `/nps/${ surveyName }`,
method: 'POST',
body: {
score,
dismissed,
feedback,
},
} );

return response;
},
} );
}

export default useSubmitNpsSurveyMutation;
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { waitFor } from '@testing-library/dom';
import { renderHook } from '@testing-library/react';
import React from 'react';
import wpcomRequest from 'wpcom-proxy-request';
import { NpsEligibilityApiResponse } from '../../types';
import useCheckNpsSurveyEligibility from '../use-check-nps-survey-eligibility';

jest.mock( 'wpcom-proxy-request', () => ( {
__esModule: true,
default: jest.fn(),
} ) );

const queryClient = new QueryClient();
const wrapper = ( { children } ) => (
<QueryClientProvider client={ queryClient }>{ children }</QueryClientProvider>
);

// TODO
// There are more cases can be covered here. e.g. non-eligible on success, a failed query, etc.
describe( 'use-check-nps-survey-eligibility', () => {
beforeEach( () => {
jest.clearAllMocks();
} );

test( 'returns eligible on a successful query', async () => {
( wpcomRequest as jest.Mock ).mockImplementation( (): Promise< NpsEligibilityApiResponse > => {
return Promise.resolve( {
display_survey: true,
seconds_until_eligible: 0,
has_available_concierge_sessions: false,
} );
} );

const { result } = renderHook( () => useCheckNpsSurveyEligibility( 'test-survey-name' ), {
wrapper,
} );

await waitFor( () => {
expect( result.current.isSuccess ).toBe( true );
expect( result.current.data ).toEqual( {
displaySurvey: true,
secondsUntilEligible: 0,
hasAvailableConciergeSessions: false,
} );
} );
} );
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useQuery, type UseQueryResult } from '@tanstack/react-query';
import wpcomRequest from 'wpcom-proxy-request';
import { NpsEligibility, NpsEligibilityApiResponse } from '../types';

function useCheckNpsSurveyEligibility( surveyName: string ): UseQueryResult< NpsEligibility > {
return useQuery( {
queryKey: [ surveyName ],
queryFn: async (): Promise< NpsEligibility > => {
const response: NpsEligibilityApiResponse = await wpcomRequest( {
path: '/nps',
query: surveyName,
apiVersion: '1.2',
} );

return {
displaySurvey: response.display_survey,
secondsUntilEligible: response.seconds_until_eligible,
hasAvailableConciergeSessions: response.has_available_concierge_sessions,
};
},
} );
}

export default useCheckNpsSurveyEligibility;
24 changes: 24 additions & 0 deletions packages/marketing/src/nps/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// TBD
// Currently the following data structures simply adhere the response of the current endpoint.
// However, it likely can be simplified.
export interface NpsEligibility {
displaySurvey: boolean;
secondsUntilEligible: number;
hasAvailableConciergeSessions: boolean;
}

export interface NpsEligibilityApiResponse {
display_survey: boolean;
seconds_until_eligible: number;
has_available_concierge_sessions: boolean;
}

export interface SubmitNpsSurveyResponse {
result: boolean;
}

export interface SubmitNpsSurveyParams {
score?: number;
dismissed?: boolean;
feedback?: string;
}
7 changes: 7 additions & 0 deletions packages/marketing/tsconfig-cjs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "./tsconfig",
"compilerOptions": {
"module": "commonjs",
"outDir": "dist/cjs"
}
}
11 changes: 11 additions & 0 deletions packages/marketing/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "@automattic/calypso-typescript-config/ts-package.json",
"compilerOptions": {
"declarationDir": "dist/types",
"outDir": "dist/esm",
"rootDir": "src",
"types": []
},
"include": [ "src", "src/*.json" ],
"exclude": [ "**/__tests__/*", "**/__mocks__/*" ]
}
1 change: 1 addition & 0 deletions packages/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
{ "path": "./languages" },
{ "path": "./launchpad" },
{ "path": "./launchpad-navigator" },
{ "path": "./marketing" },
{ "path": "./mini-cart" },
{ "path": "./odie-client" },
{ "path": "./onboarding" },
Expand Down
39 changes: 39 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,25 @@ __metadata:
languageName: unknown
linkType: soft

"@automattic/marketing@workspace:packages/marketing":
version: 0.0.0-use.local
resolution: "@automattic/marketing@workspace:packages/marketing"
dependencies:
"@automattic/calypso-build": "workspace:^"
"@automattic/calypso-storybook": "workspace:^"
"@automattic/calypso-typescript-config": "workspace:^"
"@storybook/cli": "npm:^7.6.19"
"@storybook/react": "npm:^7.6.19"
typescript: "npm:^4.7.4"
webpack: "npm:^5.94.0"
peerDependencies:
postcss: ^8.4.5
react: ^18.3.1
react-dom: ^18.3.1
tslib: ^2.3.0
languageName: unknown
linkType: soft

"@automattic/material-design-icons@workspace:^, @automattic/material-design-icons@workspace:packages/material-design-icons":
version: 0.0.0-use.local
resolution: "@automattic/material-design-icons@workspace:packages/material-design-icons"
Expand Down Expand Up @@ -33212,6 +33231,16 @@ __metadata:
languageName: node
linkType: hard

"typescript@npm:^4.7.4":
version: 4.9.5
resolution: "typescript@npm:4.9.5"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 5f6cad2e728a8a063521328e612d7876e12f0d8a8390d3b3aaa452a6a65e24e9ac8ea22beb72a924fd96ea0a49ea63bb4e251fb922b12eedfb7f7a26475e5c56
languageName: node
linkType: hard

"typescript@npm:^5.3.3":
version: 5.3.3
resolution: "typescript@npm:5.3.3"
Expand All @@ -33222,6 +33251,16 @@ __metadata:
languageName: node
linkType: hard

"typescript@patch:typescript@npm%3A^4.7.4#optional!builtin<compat/typescript>":
version: 4.9.5
resolution: "typescript@patch:typescript@npm%3A4.9.5#optional!builtin<compat/typescript>::version=4.9.5&hash=289587"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: e3333f887c6829dfe0ab6c1dbe0dd1e3e2aeb56c66460cb85c5440c566f900c833d370ca34eb47558c0c69e78ced4bfe09b8f4f98b6de7afed9b84b8d1dd06a1
languageName: node
linkType: hard

"typescript@patch:typescript@npm%3A^5.3.3#optional!builtin<compat/typescript>":
version: 5.3.3
resolution: "typescript@patch:typescript@npm%3A5.3.3#optional!builtin<compat/typescript>::version=5.3.3&hash=e012d7"
Expand Down
Loading