Skip to content

Commit c9eb644

Browse files
eugene-manuiloveugene-manuilov
authored
Merge pull request #11998 from google/enhancement/11856-improve-frequency-selector
Update PUE FrequencySelector current subscription state.
2 parents efc3cfd + 21264bc commit c9eb644

17 files changed

Lines changed: 433 additions & 232 deletions

File tree

‎assets/js/components/email-reporting/FrequencySelector.js‎

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
} from '@/js/googlesitekit/datastore/user/constants';
4040
import { Fragment } from 'react';
4141
import Typography from '@/js/components/Typography';
42-
import Tick from '@/svg/icons/tick.svg';
42+
import P from '@/js/components/Typography/P';
4343

4444
export default function FrequencySelector( { isUserSubscribed } ) {
4545
const DAY_NAMES = useMemo(
@@ -118,6 +118,12 @@ export default function FrequencySelector( { isUserSubscribed } ) {
118118
}
119119
}
120120

121+
// Determine which column the saved frequency is in (0, 1, or 2).
122+
const savedFrequencyIndex =
123+
EMAIL_REPORT_FREQUENCIES.indexOf( savedFrequency );
124+
const showCurrentSubscription =
125+
isUserSubscribed && savedFrequencyIndex !== -1;
126+
121127
return (
122128
<Fragment>
123129
<Typography
@@ -128,6 +134,29 @@ export default function FrequencySelector( { isUserSubscribed } ) {
128134
>
129135
{ __( 'Frequency', 'google-site-kit' ) }
130136
</Typography>
137+
{ showCurrentSubscription && (
138+
<div className="googlesitekit-frequency-selector__badge-row">
139+
{ EMAIL_REPORT_FREQUENCIES.map(
140+
( reportFrequency, index ) => (
141+
<div
142+
key={ reportFrequency }
143+
className="googlesitekit-frequency-selector__badge-cell"
144+
>
145+
{ index === savedFrequencyIndex && (
146+
<div className="googlesitekit-frequency-selector__current-subscription">
147+
<P>
148+
{ __(
149+
'Current subscription',
150+
'google-site-kit'
151+
) }
152+
</P>
153+
</div>
154+
) }
155+
</div>
156+
)
157+
) }
158+
</div>
159+
) }
131160
<div
132161
className="googlesitekit-frequency-selector"
133162
role="radiogroup"
@@ -159,42 +188,24 @@ export default function FrequencySelector( { isUserSubscribed } ) {
159188
}
160189
>
161190
<div className="googlesitekit-frequency-selector__label-row">
162-
<Typography
163-
className="googlesitekit-frequency-selector__label"
164-
type="label"
165-
size="large"
166-
as="div"
167-
>
168-
{ label }
191+
<div className="googlesitekit-frequency-selector__label">
192+
<Typography type="label" size="large">
193+
{ label }
194+
</Typography>
195+
</div>
196+
</div>
197+
198+
<div className="googlesitekit-frequency-selector__period">
199+
<Typography type="body" size="small">
200+
{ period }
169201
</Typography>
170-
{ savedFrequency === reportFrequency &&
171-
isUserSubscribed && (
172-
<div className="googlesitekit-frequency-selector__saved-indicator">
173-
<Tick
174-
className="googlesitekit-frequency-selector__label-tick"
175-
aria-hidden="true"
176-
/>
177-
</div>
178-
) }
179202
</div>
180203

181-
<Typography
182-
className="googlesitekit-frequency-selector__period"
183-
type="body"
184-
size="small"
185-
as="div"
186-
>
187-
{ period }
188-
</Typography>
189-
190-
<Typography
191-
className="googlesitekit-frequency-selector__description"
192-
type="body"
193-
size="small"
194-
as="div"
195-
>
196-
{ description }
197-
</Typography>
204+
<div className="googlesitekit-frequency-selector__description">
205+
<Typography type="body" size="small">
206+
{ description }
207+
</Typography>
208+
</div>
198209
</div>
199210
);
200211
} ) }

‎assets/js/components/email-reporting/FrequencySelector.stories.js‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,3 @@ PreviouslySavedFrequency.args = {
121121
},
122122
};
123123
PreviouslySavedFrequency.scenario = {};
124-
125-
export const PreviouslySavedFrequencySameAsCurrent = Template.bind( {} );
126-
PreviouslySavedFrequencySameAsCurrent.args = {
127-
isUserSubscribed: true,
128-
savedSettings: {
129-
frequency: 'monthly',
130-
},
131-
clientSettings: {
132-
frequency: 'monthly',
133-
},
134-
};
135-
PreviouslySavedFrequencySameAsCurrent.scenario = {};

‎assets/js/components/email-reporting/FrequencySelector.test.js‎

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -108,79 +108,73 @@ describe( 'FrequencySelector', () => {
108108
expect( containerElement ).toMatchSnapshot();
109109
} );
110110

111-
it( 'Previously saved frequency (saved monthly, current weekly) indicates saved on monthly only and matches snapshot', () => {
111+
it( 'Previously saved frequency (saved monthly, current weekly) shows current subscription pill above monthly card and matches snapshot', () => {
112112
setupRegistry( registry, {
113113
startOfWeek: 1,
114114
frequency: 'weekly',
115115
savedFrequency: 'monthly',
116116
} );
117117

118-
const { containerElement, getByText } = renderSelector( registry, {
119-
isUserSubscribed: true,
120-
} );
121-
122-
const indicators = containerElement.querySelectorAll(
123-
'.googlesitekit-frequency-selector__saved-indicator'
118+
const { container, containerElement, getByText } = renderSelector(
119+
registry,
120+
{
121+
isUserSubscribed: true,
122+
}
124123
);
125-
expect( indicators.length ).toBe( 1 );
126124

127-
const monthlyLabel = getByText( 'Monthly' );
128-
const monthlyCard = monthlyLabel.closest(
129-
'.googlesitekit-frequency-selector__card'
125+
// Check that there's exactly one current subscription pill.
126+
const pills = container.querySelectorAll(
127+
'.googlesitekit-frequency-selector__current-subscription'
130128
);
131-
expect( monthlyCard ).toBeTruthy();
132-
expect(
133-
monthlyCard.querySelector(
134-
'.googlesitekit-frequency-selector__saved-indicator'
135-
)
136-
).toBeInTheDocument();
129+
expect( pills.length ).toBe( 1 );
137130

138-
const weeklyLabel = getByText( 'Weekly' );
139-
const weeklyCard = weeklyLabel.closest(
140-
'.googlesitekit-frequency-selector__card'
131+
// Check that the badge row exists and contains the pill.
132+
const badgeRow = container.querySelector(
133+
'.googlesitekit-frequency-selector__badge-row'
141134
);
135+
expect( badgeRow ).toBeInTheDocument();
142136
expect(
143-
weeklyCard.querySelector(
144-
'.googlesitekit-frequency-selector__saved-indicator'
137+
badgeRow.querySelector(
138+
'.googlesitekit-frequency-selector__current-subscription'
145139
)
146-
).not.toBeInTheDocument();
140+
).toBeInTheDocument();
147141

148-
const quarterlyLabel = getByText( 'Quarterly' );
149-
const quarterlyCard = quarterlyLabel.closest(
150-
'.googlesitekit-frequency-selector__card'
151-
);
152-
expect(
153-
quarterlyCard.querySelector(
154-
'.googlesitekit-frequency-selector__saved-indicator'
155-
)
156-
).not.toBeInTheDocument();
142+
// Check that the pill text is correct.
143+
expect( getByText( 'Current subscription' ) ).toBeInTheDocument();
157144

158145
expect( containerElement ).toMatchSnapshot();
159146
} );
160147

161-
it( 'Previously saved frequency (same as the current frequency) shows saved indicator on selected card and matches snapshot', () => {
148+
it( 'Previously saved frequency (same as the current frequency) shows current subscription pill above selected card and matches snapshot', () => {
162149
setupRegistry( registry, {
163150
startOfWeek: 1,
164151
frequency: 'monthly',
165152
savedFrequency: 'monthly',
166153
} );
167154

168-
const { containerElement, getByText } = renderSelector( registry, {
169-
isUserSubscribed: true,
170-
} );
171-
172-
const monthlyLabel = getByText( 'Monthly' );
173-
const monthlyCard = monthlyLabel.closest(
174-
'.googlesitekit-frequency-selector__card'
155+
const { container, containerElement, getByText } = renderSelector(
156+
registry,
157+
{
158+
isUserSubscribed: true,
159+
}
175160
);
176-
expect( monthlyCard ).toBeTruthy();
177161

162+
// Check that there's exactly one current subscription pill in the badge row.
163+
const badgeRow = container.querySelector(
164+
'.googlesitekit-frequency-selector__badge-row'
165+
);
166+
expect( badgeRow ).toBeInTheDocument();
178167
expect(
179-
monthlyCard.querySelector(
180-
'.googlesitekit-frequency-selector__saved-indicator'
168+
badgeRow.querySelector(
169+
'.googlesitekit-frequency-selector__current-subscription'
181170
)
182171
).toBeInTheDocument();
183172

173+
// Check that the monthly card is selected.
174+
const monthlyLabel = getByText( 'Monthly' );
175+
const monthlyCard = monthlyLabel.closest(
176+
'.googlesitekit-frequency-selector__card'
177+
);
184178
expect(
185179
monthlyCard.classList.contains(
186180
'googlesitekit-frequency-selector__card--selected'

0 commit comments

Comments
 (0)