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

Focused Launch: PlanDetails View #47373

Merged
merged 10 commits into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/data-stores/src/launch/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ const plan: Reducer< Plans.Plan | undefined, LaunchAction > = ( state, action )
return state;
};

/**
* To keep track of the last paid plan the user has picked
* This is useful information as this paid plan can be suggested later
*
* @param state the state
* @param action the action
*/
const paidPlan: Reducer< Plans.Plan | undefined, LaunchAction > = ( state, action ) => {
if ( action.type === 'SET_PLAN' && ! action.plan?.isFree ) {
return action.plan;
}
return state;
};

const isFocusedLaunchOpen: Reducer< boolean, LaunchAction > = ( state = false, action ) => {
if ( action.type === 'OPEN_FOCUSED_LAUNCH' ) {
return true;
Expand Down Expand Up @@ -135,6 +149,7 @@ const reducer = combineReducers( {
confirmedDomainSelection,
domainSearch,
plan,
paidPlan,
isSidebarOpen,
isSidebarFullscreen,
isExperimental,
Expand Down
9 changes: 9 additions & 0 deletions packages/data-stores/src/launch/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ export const getSelectedDomain = ( state: State ): DomainSuggestions.DomainSugge
state.domain;
export const getSelectedPlan = ( state: State ): Plans.Plan | undefined => state.plan;

/**
* Returns the last paid plan the user has picked.
* If they revert to a free plan,
* this is useful if you want to recommend their once-picked paid plan
*
* @param state State
*/
export const getPaidPlan = ( state: State ): Plans.Plan | undefined => state.paidPlan;

// Completion status of steps is derived from the state of the launch flow
export const isStepCompleted = ( state: State, step: LaunchStepType ): boolean => {
if ( step === LaunchStep.Plan ) {
Expand Down
1 change: 1 addition & 0 deletions packages/launch/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@automattic/data-stores": "^1.0.0-alpha.1",
"@automattic/domain-picker": "^1.0.0-alpha.0",
"@automattic/onboarding": "^1.0.0",
"@automattic/plans-grid": "^1.0.0-alpha.0",
"@wordpress/components": "^10.0.5",
"@wordpress/icons": "^2.4.0",
"@wordpress/url": "^2.17.0",
Expand Down
23 changes: 23 additions & 0 deletions packages/launch/src/focused-launch/go-back-button/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* External dependencies
*/
import React from 'react';
import { __ } from '@wordpress/i18n';
import { Icon, chevronLeft } from '@wordpress/icons';

/**
* Internal dependencies
*/
import { BackButton } from '@automattic/onboarding';
import './style.scss';

declare const __i18n_text_domain__: string;

export default function GoBackButton( props ) {
return (
<BackButton { ...props }>
<Icon icon={ chevronLeft } />
{ __( 'Go back', __i18n_text_domain__ ) }
</BackButton>
);
}
22 changes: 22 additions & 0 deletions packages/launch/src/focused-launch/go-back-button/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
.go-back-button__focused-launch {
text-decoration: underline;
background: transparent;
border: none;
padding: 0;
margin: 0;
margin-bottom: 24px;
cursor: pointer;

& > * {
vertical-align: middle;
}

svg {
// the svg has 5 empty pixels on the left
margin-left: -5px;
}
}

.go-back-button__focused-launch:hover {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Non-blocker: Not a big deal just me being picky, but we could move this into the above selector to avoid repeating the selector.

.go-back-button__focused-launch {
	//...

	&:hover {
		opacity: 0.7;
	}
}

opacity: 0.7;
}
70 changes: 65 additions & 5 deletions packages/launch/src/focused-launch/plan-details/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,82 @@
/**
* External dependencies
*/
import React from 'react';
import { Link } from 'react-router-dom';
import * as React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what are the benefits of import * as React over import React ?

Copy link
Contributor Author

@tjcafferkey tjcafferkey Nov 18, 2020

Choose a reason for hiding this comment

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

I thought this. Looks like it's being used for the functional component typings but not sure the benefit of *

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it would work anyway

Choose a reason for hiding this comment

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

I know we used this way of importing for a while for performance reasons. I believe now we have webpack configured so this isn't necessary anymore. Pinging @sirreal to confirm though 🙏

Copy link
Member

Choose a reason for hiding this comment

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

If you do import React... you need allowSyntheticDefaultImports = true in tsconfig. import * as React is more semantically accurate and makes TypeScript happy. But don't quote me on this. I think (not sure) I saw @sirreal do it, and he is my TypeScript prophet.

Copy link
Member

Choose a reason for hiding this comment

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

There were some perf concerns and I don't know whether they're relevant. Either way, I think we have conclusive evidence that import * as React from 'react'; is "right": facebook/react#18102

[import * as React from "react";] is the correct way to import React from an ES module since the ES module will not have a default export. Only named exports.

I'm doing some changes to named exports and this will stop working. For open source ES modules we'll probably publish some default object that warns as an upgrade path but we can't use that in our own code.

We could add a lint rule for this and/or codemod it, but I don't know whether there is significant impact beyond consistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation! I learnt something new

Copy link
Member

Choose a reason for hiding this comment

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

If you do import React... you need allowSyntheticDefaultImports = true in tsconfig. import * as React is more semantically accurate and makes TypeScript happy.

Yes, I think this is right. Note, this isn't really about TypeScript but about ECMAScript modules which are quirky. I wrote a bit here (p4TIVU-8Lf-p2).

import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { Plans } from '@automattic/data-stores';
import PlansGrid from '@automattic/plans-grid';
import { Title, SubTitle } from '@automattic/onboarding';
import { useHistory } from 'react-router-dom';

/**
* Internal dependencies
*/
import { Route } from '../route';
import { LAUNCH_STORE } from '../../stores';
import GoBackButton from '../go-back-button';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use Back Button from @automattic/onboarding? Seen here

Copy link

@razvanpapadopol razvanpapadopol Nov 18, 2020

Choose a reason for hiding this comment

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

An idea would be that GoBackButton should use BackButton and then reuse here and in DomainDetails View. See also #47497 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Great idea. Didn't know about this one.


import './style.scss';

const PlanDetails: React.FunctionComponent = () => {
const domain = useSelect( ( select ) => select( LAUNCH_STORE ).getSelectedDomain() );
const selectedPlan = useSelect( ( select ) => select( LAUNCH_STORE ).getSelectedPlan() );
const history = useHistory();

const { updatePlan } = useDispatch( LAUNCH_STORE );

const hasPaidDomain = domain && ! domain.is_free;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If domain is null or undefined you will get these values assigned to hasPaidDomain and it'll be coerced into a falsely value.

If you changed const hasPaidDomain = domain && ! domain.is_free; to const hasPaidDomain = domain?.is_free === false; then you'll get a more predictable boolean returned.

Not a blocker though, as it'll be coerced into a falsely value anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Great idea but Typescript won't like because is_free is typed wrong.


const handleSelect = ( planSlug: Plans.PlanSlug ) => {
updatePlan( planSlug );
history.goBack();
};

const goBackToSummary = () => {
history.goBack();
};

return (
<div>
<Link to={ Route.Summary }>{ __( 'Go back', __i18n_text_domain__ ) }</Link>
<p>{ __( 'Select a plan', __i18n_text_domain__ ) }</p>
<div className="focused-launch-plan-details__back-button-wrapper">
<GoBackButton onClick={ goBackToSummary } />
</div>
<div className="focused-launch-plan-details__header">
<div>
<Title>{ __( 'Select a plan', __i18n_text_domain__ ) }</Title>
<SubTitle>
{ __(
"There's no risk, you can cancel for a full refund within 30 days.",
__i18n_text_domain__
) }
</SubTitle>
</div>
</div>
<div className="focused-launch-plan-details__body">
<PlansGrid
currentDomain={ domain }
onPlanSelect={ handleSelect }
currentPlan={ selectedPlan }
onPickDomainClick={ goBackToSummary }
customTagLines={ {
free_plan: __( 'Best for getting started', __i18n_text_domain__ ),
'business-bundle': __( 'Best for small businesses', __i18n_text_domain__ ),
} }
showPlanTaglines
popularBadgeVariation="NEXT_TO_NAME"
disabledPlans={
hasPaidDomain
? {
[ Plans.PLAN_FREE ]: __(
'Not available with custom domain',
__i18n_text_domain__
),
}
: undefined
}
CTAVariation="FULL_WIDTH"
locale="user"
/>
</div>
</div>
);
};
Expand Down
3 changes: 3 additions & 0 deletions packages/launch/src/focused-launch/plan-details/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.focused-launch-plan-details__back-button-wrapper {
margin-bottom: 24px;
}
50 changes: 18 additions & 32 deletions packages/launch/src/focused-launch/summary/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import FocusedLaunchSummaryItem, {
*/
import { Route } from '../route';
import { useTitle, useDomainSearch, useSiteDomains, useSite, usePlans } from '../../hooks';
import { LAUNCH_STORE, Plan, SiteDetailsPlan } from '../../stores';
import { LAUNCH_STORE } from '../../stores';
import LaunchContext from '../../context';
import { isDefaultSiteTitle } from '../../utils';

Expand Down Expand Up @@ -245,39 +245,35 @@ type PlanStepProps = CommonStepProps & {
hasPaidPlan?: boolean;
hasPaidDomain?: boolean;
selectedPaidDomain?: boolean;
defaultPaidPlan: Plan | undefined;
defaultFreePlan: Plan | undefined;
planPrices: Record< string, string >;
selectedPlan: Plan | undefined;
onSetPlan: ( plan: Plan ) => void;
onUnsetPlan: () => void;
sitePlan: SiteDetailsPlan | undefined;
};

const PlanStep: React.FunctionComponent< PlanStepProps > = ( {
stepIndex,
hasPaidPlan = false,
hasPaidDomain = false,
selectedPaidDomain = false,
defaultPaidPlan,
defaultFreePlan,
planPrices,
selectedPlan,
onSetPlan,
onUnsetPlan,
sitePlan,
} ) => {
const { setPlan, unsetPlan } = useDispatch( LAUNCH_STORE );

const selectedPlan = useSelect( ( select ) => select( LAUNCH_STORE ).getSelectedPlan() );

const onceSelectedPaidPlan = useSelect( ( select ) => select( LAUNCH_STORE ).getPaidPlan() );

const { defaultPaidPlan, defaultFreePlan, planPrices } = usePlans();

const sitePlan = useSite().sitePlan;

useEffect( () => {
// To keep the launch store state valid,
// unselect the free plan if the user selected a paid domain.
// free plans don't support paid domains.
if ( selectedPaidDomain && selectedPlan && selectedPlan.isFree ) {
onUnsetPlan();
unsetPlan();
}
}, [ selectedPaidDomain, selectedPlan, onUnsetPlan ] );
}, [ selectedPaidDomain, selectedPlan, unsetPlan ] );

// if the user picks up a paid plan from the detailed plan page, show it, otherwise show premium plan
const paidPlan = selectedPlan && ! selectedPlan.isFree ? selectedPlan : defaultPaidPlan;
// if the user picks (or ever picked) up a paid plan from the detailed plan page, show it, otherwise show premium plan
const paidPlan = onceSelectedPaidPlan || defaultPaidPlan;

return (
<SummaryStep
Expand Down Expand Up @@ -339,7 +335,7 @@ const PlanStep: React.FunctionComponent< PlanStepProps > = ( {
<FocusedLaunchSummaryItem
isLoading={ ! defaultFreePlan || ! defaultPaidPlan }
readOnly={ hasPaidDomain || selectedPaidDomain }
onClick={ () => defaultFreePlan && onSetPlan( defaultFreePlan ) }
onClick={ () => defaultFreePlan && setPlan( defaultFreePlan ) }
isSelected={ ! ( hasPaidDomain || selectedPaidDomain ) && selectedPlan?.isFree }
>
<LeadingContentSide
Expand All @@ -358,7 +354,7 @@ const PlanStep: React.FunctionComponent< PlanStepProps > = ( {
</FocusedLaunchSummaryItem>
<FocusedLaunchSummaryItem
isLoading={ ! defaultFreePlan || ! defaultPaidPlan }
onClick={ () => paidPlan && onSetPlan( paidPlan ) }
onClick={ () => paidPlan && setPlan( paidPlan ) }
isSelected={ selectedPlan?.storeSlug === paidPlan?.storeSlug }
>
<LeadingContentSide
Expand Down Expand Up @@ -439,11 +435,9 @@ const Summary: React.FunctionComponent = () => {
const { title, updateTitle, saveTitle, isSiteTitleStepVisible, showSiteTitleStep } = useTitle();

const { sitePrimaryDomain, siteSubdomain, hasPaidDomain } = useSiteDomains();
const selectedPlan = useSelect( ( select ) => select( LAUNCH_STORE ).getSelectedPlan() );
const selectedDomain = useSelect( ( select ) => select( LAUNCH_STORE ).getSelectedDomain() );
const { setDomain, unsetDomain, setPlan, unsetPlan } = useDispatch( LAUNCH_STORE );
const { setDomain, unsetDomain } = useDispatch( LAUNCH_STORE );
const domainSearch = useDomainSearch();
const { defaultPaidPlan, defaultFreePlan, planPrices } = usePlans();

const site = useSite();

Expand All @@ -466,7 +460,6 @@ const Summary: React.FunctionComponent = () => {
}
}, [ title, showSiteTitleStep, isSiteTitleStepVisible ] );

const sitePlan = site.sitePlan;
const hasPaidPlan = site.isPaidPlan;

// Prepare Steps
Expand Down Expand Up @@ -506,13 +499,6 @@ const Summary: React.FunctionComponent = () => {
hasPaidDomain={ hasPaidDomain }
stepIndex={ forwardStepIndex ? stepIndex : undefined }
key={ stepIndex }
defaultPaidPlan={ defaultPaidPlan }
defaultFreePlan={ defaultFreePlan }
selectedPlan={ selectedPlan }
onSetPlan={ setPlan }
onUnsetPlan={ unsetPlan }
planPrices={ planPrices }
sitePlan={ sitePlan }
/>
);

Expand Down
3 changes: 2 additions & 1 deletion packages/launch/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
{ "path": "../data-stores" },
{ "path": "../components" },
{ "path": "../onboarding" },
{ "path": "../domain-picker" }
{ "path": "../domain-picker" },
{ "path": "../plans-grid" }
]
}
14 changes: 14 additions & 0 deletions packages/plans-grid/src/plans-grid/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import debugFactory from 'debug';
import PlansTable from '../plans-table';
import PlansAccordion from '../plans-accordion';
import PlansDetails from '../plans-details';
import type { CTAVariation, CustomTagLinesMap, PopularBadgeVariation } from '../plans-table/types';
export type { CTAVariation, CustomTagLinesMap, PopularBadgeVariation } from '../plans-table/types';

/**
* Style dependencies
Expand All @@ -34,6 +36,10 @@ export interface Props {
disabledPlans?: { [ planSlug: string ]: string };
isAccordion?: boolean;
locale: string;
showPlanTaglines?: boolean;
CTAVariation?: CTAVariation;
popularBadgeVariation?: PopularBadgeVariation;
customTagLines?: CustomTagLinesMap;
}

const PlansGrid: React.FunctionComponent< Props > = ( {
Expand All @@ -46,6 +52,10 @@ const PlansGrid: React.FunctionComponent< Props > = ( {
disabledPlans,
isAccordion,
locale,
showPlanTaglines = false,
CTAVariation = 'NORMAL',
popularBadgeVariation = 'ON_TOP',
Comment on lines +56 to +57

Choose a reason for hiding this comment

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

I don't think we need to maintain another variation of the PlansGrid in a non-accordion mode. However, it's non blocking for this PR.

Just pinging @ollierozdarz to confirm so we can clean up later. Here is the old table version with badge on top and CTA non full-width.
Screenshot 2020-11-18 at 13 37 36

customTagLines,
} ) => {
isAccordion && debug( 'PlansGrid accordion version is active' );

Expand All @@ -67,12 +77,16 @@ const PlansGrid: React.FunctionComponent< Props > = ( {
></PlansAccordion>
) : (
<PlansTable
popularBadgeVariation={ popularBadgeVariation }
CTAVariation={ CTAVariation }
selectedPlanSlug={ currentPlan?.storeSlug ?? '' }
onPlanSelect={ onPlanSelect }
customTagLines={ customTagLines }
currentDomain={ currentDomain }
onPickDomainClick={ onPickDomainClick }
disabledPlans={ disabledPlans }
locale={ locale }
showTaglines={ showPlanTaglines }
></PlansTable>
) }
</div>
Expand Down
Loading