-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Changes from all commits
3a4dd0e
e19a532
75f6bb7
4a25795
dd219e6
0958374
6d6113c
1e4e108
c735aad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require( '@automattic/calypso-storybook' )(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
## 1.0.0 | ||
|
||
The initial version. |
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 |
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)' ], | ||
}; |
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.", | ||
"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" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default as Nps } from './nps'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Nps | ||
|
||
The NPS(Net Promoter Score) survey form. |
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' }, | ||
}, | ||
}; |
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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Which brings up the question whether |
||
<span className="label-left">Not likely</span> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly. Could be |
||
<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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are missing these dependencies in |
||
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, | ||
} ); | ||
} ); | ||
} ); | ||
} ); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have we considered adding a |
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; |
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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"extends": "./tsconfig", | ||
"compilerOptions": { | ||
"module": "commonjs", | ||
"outDir": "dist/cjs" | ||
} | ||
} |
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__/*" ] | ||
} |
There was a problem hiding this comment.
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.