From 685ef88e911463190a061d6b4f10c12a25b13133 Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Fri, 31 May 2024 13:45:30 -0400 Subject: [PATCH 1/6] Modify limited availability banner display logic --- .../e2e/core/linodes/plan-selection.spec.ts | 62 +++++++++++-------- .../KubernetesPlansPanel.tsx | 3 +- .../PlansPanel/PlanInformation.test.tsx | 4 +- .../components/PlansPanel/PlanInformation.tsx | 26 ++++---- .../components/PlansPanel/PlansPanel.tsx | 3 +- .../features/components/PlansPanel/utils.ts | 1 - 6 files changed, 54 insertions(+), 45 deletions(-) diff --git a/packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts b/packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts index 86838505220..5a6480c18ec 100644 --- a/packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts +++ b/packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts @@ -44,6 +44,11 @@ const mockDedicatedLinodeTypes = [ label: 'dedicated-3', class: 'dedicated', }), + linodeTypeFactory.build({ + id: 'dedicated-4', + label: 'dedicated-4', + class: 'dedicated', + }), ]; const mockSharedLinodeTypes = [ @@ -98,11 +103,21 @@ const mockRegionAvailability = [ available: false, region: 'us-east', }), + regionAvailabilityFactory.build({ + plan: 'dedicated-4', + available: false, + region: 'us-east', + }), regionAvailabilityFactory.build({ plan: 'highmem-1', available: false, region: 'us-east', }), + regionAvailabilityFactory.build({ + plan: 'shared-3', + available: false, + region: 'us-east', + }), ]; const linodePlansPanel = '[data-qa-tp="Linode Plan"]'; @@ -110,7 +125,7 @@ const k8PlansPanel = '[data-qa-tp="Add Node Pools"]'; const planSelectionTable = 'List of Linode Plans'; const notices = { - limitedAvailability: '[data-testid="disabled-plan-tooltip"]', + limitedAvailability: '[data-testid="limited-availability-banner"]', unavailable: '[data-testid="notice-error"]', }; @@ -136,15 +151,15 @@ describe('displays linode plans panel based on availability', () => { // Dedicated CPU tab // Should be selected/open by default // Should have the limited availability notice - // Should contain 4 plans (5 rows including the header row) - // Should have 2 plans disabled - // Should have tooltips for the disabled plans (not more than half disabled plans in the panel) + // Should contain 5 plans (6 rows including the header row) + // Should have 3 plans disabled + // Should not have tooltips for the disabled plans (more than half disabled plans in the panel) cy.get(linodePlansPanel).within(() => { cy.findAllByRole('alert').should('have.length', 1); cy.get(notices.limitedAvailability).should('be.visible'); cy.findByRole('table', { name: planSelectionTable }).within(() => { - cy.findAllByRole('row').should('have.length', 5); + cy.findAllByRole('row').should('have.length', 6); cy.get('[id="dedicated-1"]').should('be.enabled'); cy.get('[id="dedicated-2"]').should('be.enabled'); cy.get( @@ -152,15 +167,15 @@ describe('displays linode plans panel based on availability', () => { ); cy.get('[id="dedicated-3"]').should('be.disabled'); cy.get('[id="g6-dedicated-64"]').should('be.disabled'); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 2); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); }); }); // Shared CPU tab // Should have no notices // Should contain 3 plans (4 rows including the header row) - // Should have 0 disabled plan - // Should have no tooltip for the disabled plan + // Should have 1 disabled plan + // Should have one tooltip for the disabled plan cy.findByText('Shared CPU').click(); cy.get(linodePlansPanel).within(() => { cy.findAllByRole('alert').should('have.length', 0); @@ -169,8 +184,8 @@ describe('displays linode plans panel based on availability', () => { cy.findAllByRole('row').should('have.length', 4); cy.get('[id="shared-1"]').should('be.enabled'); cy.get('[id="shared-2"]').should('be.enabled'); - cy.get('[id="shared-3"]').should('be.enabled'); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); + cy.get('[id="shared-3"]').should('be.disabled'); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 1); }); }); @@ -178,7 +193,7 @@ describe('displays linode plans panel based on availability', () => { // Should have the limited availability notice // Should contain 1 plan (2 rows including the header row) // Should have one disabled plan - // Should have tooltip for the disabled plan (more than half disabled plans in the panel, but only one plan) + // Should have no tooltip for the disabled plan (more than half disabled plans in the panel) cy.findByText('High Memory').click(); cy.get(linodePlansPanel).within(() => { cy.findAllByRole('alert').should('have.length', 1); @@ -187,7 +202,7 @@ describe('displays linode plans panel based on availability', () => { cy.findByRole('table', { name: planSelectionTable }).within(() => { cy.findAllByRole('row').should('have.length', 2); cy.get('[id="highmem-1"]').should('be.disabled'); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 1); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); }); }); @@ -232,9 +247,9 @@ describe('displays kubernetes plans panel based on availability', () => { // Dedicated CPU tab // Should be selected/open by default // Should have the limited availability notice - // Should contain 4 plans (5 rows including the header row) - // Should have 2 plans disabled - // Should have tooltips for the disabled plans (not more than half disabled plans in the panel) + // Should contain 5 plans (6 rows including the header row) + // Should have 3 plans disabled + // Should have no tooltips for the disabled plans (more than half disabled plans in the panel) // All inputs for a row should be enabled if row is enabled (only testing one row in suite) // All inputs for a disabled row should be disabled (only testing one row in suite) cy.get(k8PlansPanel).within(() => { @@ -242,7 +257,7 @@ describe('displays kubernetes plans panel based on availability', () => { cy.get(notices.limitedAvailability).should('be.visible'); cy.findByRole('table', { name: planSelectionTable }).within(() => { - cy.findAllByRole('row').should('have.length', 5); + cy.findAllByRole('row').should('have.length', 6); cy.get('[data-qa-plan-row="dedicated-1"]').should( 'not.have.attr', 'disabled' @@ -270,14 +285,14 @@ describe('displays kubernetes plans panel based on availability', () => { ) .should('be.disabled'); }); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 2); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); }); }); // Shared CPU tab // Should have no notices // Should contain 3 plans (4 rows including the header row) - // Should have 1 disabled plan + // Should have 2 disabled plans // Should have tooltip for the disabled plan (not more than half disabled plans in the panel) cy.findByText('Shared CPU').click(); cy.get(k8PlansPanel).within(() => { @@ -293,11 +308,8 @@ describe('displays kubernetes plans panel based on availability', () => { 'not.have.attr', 'disabled' ); - cy.get('[data-qa-plan-row="shared-3"]').should( - 'not.have.attr', - 'disabled' - ); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); + cy.get('[data-qa-plan-row="shared-3"]').should('have.attr', 'disabled'); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 1); }); }); @@ -305,7 +317,7 @@ describe('displays kubernetes plans panel based on availability', () => { // Should have the limited availability notice // Should contain 1 plan (2 rows including the header row) // Should have one disabled plan - // Should have tooltip for the disabled plan (more than half disabled plans in the panel, but only one plan) + // Should have no tooltip for the disabled plan (more than half disabled plans in the panel) cy.findByText('High Memory').click(); cy.get(k8PlansPanel).within(() => { cy.findAllByRole('alert').should('have.length', 1); @@ -317,7 +329,7 @@ describe('displays kubernetes plans panel based on availability', () => { 'have.attr', 'disabled' ); - cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 1); + cy.findAllByTestId('disabled-plan-tooltip').should('have.length', 0); }); }); diff --git a/packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlansPanel.tsx b/packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlansPanel.tsx index 6660f315360..fffc1efc25b 100644 --- a/packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlansPanel.tsx +++ b/packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlansPanel.tsx @@ -76,7 +76,6 @@ export const KubernetesPlansPanel = (props: Props) => { const plansMap: PlanSelectionType[] = plans[plan]; const { allDisabledPlans, - hasDisabledPlans, hasMajorityOfPlansDisabled, plansForThisLinodeTypeClass, } = extractPlansInformation({ @@ -94,7 +93,7 @@ export const KubernetesPlansPanel = (props: Props) => { isSelectedRegionEligibleForPlan={isSelectedRegionEligibleForPlan( plan )} - hasDisabledPlans={hasDisabledPlans} + hasMajorityOfPlansDisabled={hasMajorityOfPlansDisabled} hasSelectedRegion={hasSelectedRegion} planType={plan} regionsData={regionsData} diff --git a/packages/manager/src/features/components/PlansPanel/PlanInformation.test.tsx b/packages/manager/src/features/components/PlansPanel/PlanInformation.test.tsx index a48c0fcff31..089a59d27ad 100644 --- a/packages/manager/src/features/components/PlansPanel/PlanInformation.test.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlanInformation.test.tsx @@ -11,7 +11,7 @@ import { import type { PlanInformationProps } from './PlanInformation'; const mockProps: PlanInformationProps = { - hasDisabledPlans: false, + hasMajorityOfPlansDisabled: false, hasSelectedRegion: true, isSelectedRegionEligibleForPlan: false, planType: 'standard', @@ -38,7 +38,7 @@ describe('PlanInformation', () => { renderWithTheme( diff --git a/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx b/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx index 399074560ce..4943aa1ad3b 100644 --- a/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx @@ -22,7 +22,7 @@ import type { Region } from '@linode/api-v4'; export interface PlanInformationProps { disabledClasses?: LinodeTypeClass[]; - hasDisabledPlans: boolean; + hasMajorityOfPlansDisabled: boolean; hasSelectedRegion: boolean; hideLimitedAvailabilityBanner?: boolean; isSelectedRegionEligibleForPlan: boolean; @@ -34,7 +34,7 @@ export const PlanInformation = (props: PlanInformationProps) => { const theme = useTheme(); const { disabledClasses, - hasDisabledPlans, + hasMajorityOfPlansDisabled, hasSelectedRegion, hideLimitedAvailabilityBanner, isSelectedRegionEligibleForPlan, @@ -74,7 +74,7 @@ export const PlanInformation = (props: PlanInformationProps) => { isSelectedRegionEligibleForPlan && !hideLimitedAvailabilityBanner && ( )} @@ -91,21 +91,21 @@ export const PlanInformation = (props: PlanInformationProps) => { export const limitedAvailabilityBannerTestId = 'limited-availability-banner'; interface LimitedAvailabilityNoticeProps { - hasDisabledPlans: boolean; + hasMajorityOfPlansDisabled: boolean; planType: 'shared' | LinodeTypeClass; } export const LimitedAvailabilityNotice = ( props: LimitedAvailabilityNoticeProps ) => { - const { hasDisabledPlans, planType } = props; + const { hasMajorityOfPlansDisabled, planType } = props; switch (planType) { case 'dedicated': return ( ); @@ -114,7 +114,7 @@ export const LimitedAvailabilityNotice = ( return ( ); @@ -123,7 +123,7 @@ export const LimitedAvailabilityNotice = ( return ( ); @@ -132,7 +132,7 @@ export const LimitedAvailabilityNotice = ( return ( ); @@ -141,7 +141,7 @@ export const LimitedAvailabilityNotice = ( return ( ); @@ -153,16 +153,16 @@ export const LimitedAvailabilityNotice = ( interface LimitedAvailabilityNoticeCopyProps { docsLink: string; - hasDisabledPlans: boolean; + hasMajorityOfPlansDisabled: boolean; planTypeLabel: string; } export const LimitedAvailabilityNoticeCopy = ( props: LimitedAvailabilityNoticeCopyProps ) => { - const { docsLink, hasDisabledPlans, planTypeLabel } = props; + const { docsLink, hasMajorityOfPlansDisabled, planTypeLabel } = props; - return hasDisabledPlans ? ( + return hasMajorityOfPlansDisabled ? ( ({ marginBottom: theme.spacing(3), diff --git a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx index b516c11e678..a0de2edfe82 100644 --- a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx @@ -119,7 +119,6 @@ export const PlansPanel = (props: PlansPanelProps) => { const plansMap: PlanSelectionType[] = plans[plan]; const { allDisabledPlans, - hasDisabledPlans, hasMajorityOfPlansDisabled, plansForThisLinodeTypeClass, } = extractPlansInformation({ @@ -144,7 +143,7 @@ export const PlansPanel = (props: PlansPanelProps) => { plan )} disabledClasses={disabledClasses} - hasDisabledPlans={hasDisabledPlans} + hasMajorityOfPlansDisabled={hasMajorityOfPlansDisabled} hasSelectedRegion={hasSelectedRegion} planType={plan} regionsData={regionsData || []} diff --git a/packages/manager/src/features/components/PlansPanel/utils.ts b/packages/manager/src/features/components/PlansPanel/utils.ts index c3699788c66..5c1081377ad 100644 --- a/packages/manager/src/features/components/PlansPanel/utils.ts +++ b/packages/manager/src/features/components/PlansPanel/utils.ts @@ -331,7 +331,6 @@ export const extractPlansInformation = ({ }, []); const hasDisabledPlans = allDisabledPlans.length > 0; const hasMajorityOfPlansDisabled = - plans.length !== 1 && allDisabledPlans.length > plansForThisLinodeTypeClass.length / 2; return { From 24175bf317e9e4ff6d6eec6292105024b1f1b297 Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Fri, 31 May 2024 14:28:49 -0400 Subject: [PATCH 2/6] Update class copy --- .../components/PlansPanel/PlanInformation.tsx | 132 ++++++------------ 1 file changed, 46 insertions(+), 86 deletions(-) diff --git a/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx b/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx index 4943aa1ad3b..2f9bd20478d 100644 --- a/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlanInformation.tsx @@ -31,7 +31,6 @@ export interface PlanInformationProps { } export const PlanInformation = (props: PlanInformationProps) => { - const theme = useTheme(); const { disabledClasses, hasMajorityOfPlansDisabled, @@ -72,111 +71,72 @@ export const PlanInformation = (props: PlanInformationProps) => { ) : null} {hasSelectedRegion && isSelectedRegionEligibleForPlan && - !hideLimitedAvailabilityBanner && ( - + !hideLimitedAvailabilityBanner && + hasMajorityOfPlansDisabled && ( + ({ + marginBottom: theme.spacing(3), + marginLeft: 0, + marginTop: 0, + padding: `${theme.spacing(0.5)} ${theme.spacing(2)}`, + })} + dataTestId={limitedAvailabilityBannerTestId} + variant="warning" + > + + These plans have limited deployment availability. + + )} - - {planTabInfoContent[planType]?.typography} - + ); }; export const limitedAvailabilityBannerTestId = 'limited-availability-banner'; -interface LimitedAvailabilityNoticeProps { - hasMajorityOfPlansDisabled: boolean; +interface ClassDescriptionCopyProps { planType: 'shared' | LinodeTypeClass; } -export const LimitedAvailabilityNotice = ( - props: LimitedAvailabilityNoticeProps -) => { - const { hasMajorityOfPlansDisabled, planType } = props; +export const ClassDescriptionCopy = (props: ClassDescriptionCopyProps) => { + const { planType } = props; + const theme = useTheme(); + let planTypeLabel: null | string; + let docLink: null | string; switch (planType) { case 'dedicated': - return ( - - ); - + planTypeLabel = 'Dedicated CPU'; + docLink = DEDICATED_COMPUTE_INSTANCES_LINK; + break; case 'shared': - return ( - - ); - + planTypeLabel = 'Shared CPU'; + docLink = SHARED_COMPUTE_INSTANCES_LINK; + break; case 'highmem': - return ( - - ); - + planTypeLabel = 'High Memory'; + docLink = HIGH_MEMORY_COMPUTE_INSTANCES_LINK; + break; case 'premium': - return ( - - ); - + planTypeLabel = 'Premium CPU'; + docLink = PREMIUM_COMPUTE_INSTANCES_LINK; + break; case 'gpu': - return ( - - ); - + planTypeLabel = 'GPU'; + docLink = GPU_COMPUTE_INSTANCES_LINK; + break; default: - return null; + planTypeLabel = null; + docLink = null; } -}; - -interface LimitedAvailabilityNoticeCopyProps { - docsLink: string; - hasMajorityOfPlansDisabled: boolean; - planTypeLabel: string; -} - -export const LimitedAvailabilityNoticeCopy = ( - props: LimitedAvailabilityNoticeCopyProps -) => { - const { docsLink, hasMajorityOfPlansDisabled, planTypeLabel } = props; - return hasMajorityOfPlansDisabled ? ( - ({ - marginBottom: theme.spacing(3), - marginLeft: 0, - marginTop: 0, - padding: `${theme.spacing(0.5)} ${theme.spacing(2)}`, - })} - dataTestId={limitedAvailabilityBannerTestId} - variant="warning" + return planTypeLabel && docLink ? ( + - - These plans have limited deployment availability.{' '} - Learn more about our {planTypeLabel} plans. - - + {planTabInfoContent[planType]?.typography}{' '} + Learn more about our {planTypeLabel} plans. + ) : null; }; From d0ada7404e8f7efc0036a2ddf0d42a26aa85d740 Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Fri, 31 May 2024 15:20:27 -0400 Subject: [PATCH 3/6] Added changeset: Modify limited availability banner display logic --- .../manager/.changeset/pr-10536-changed-1717183226858.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-10536-changed-1717183226858.md diff --git a/packages/manager/.changeset/pr-10536-changed-1717183226858.md b/packages/manager/.changeset/pr-10536-changed-1717183226858.md new file mode 100644 index 00000000000..d7d6297562e --- /dev/null +++ b/packages/manager/.changeset/pr-10536-changed-1717183226858.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Modify limited availability banner display logic ([#10536](https://github.com/linode/manager/pull/10536)) From fe6a75d2129aeaa0ae6e9fba2b50114c2c5efbfa Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Fri, 31 May 2024 15:23:08 -0400 Subject: [PATCH 4/6] Add comments --- .../src/features/components/PlansPanel/PlansPanel.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx index a0de2edfe82..d544889802c 100644 --- a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx @@ -48,6 +48,14 @@ export interface PlansPanelProps { types: PlanSelectionType[]; } +/** + * PlansPanel is a tabbed panel that displays a list of plans for a Linode. + * It is used in the Linode create, Kubernetes and Database create flows. + * It contains ample logic to determine which plans are available based on the selected region availability and display related visual indicators: + * - If the region is not supported, show an error notice and disable all plans. + * - If more than half plans are disabled, show the limited availability banner and hide the limited availability tooltip + * - If less than half plans are disabled, hide the limited availability banner and show the limited availability tooltip + */ export const PlansPanel = (props: PlansPanelProps) => { const { className, From 9ae01228a7f378afabf75fcdb589c707c72624a2 Mon Sep 17 00:00:00 2001 From: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com> Date: Wed, 5 Jun 2024 08:04:18 -0400 Subject: [PATCH 5/6] Update packages/manager/src/features/components/PlansPanel/PlansPanel.tsx Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com> --- .../manager/src/features/components/PlansPanel/PlansPanel.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx index d544889802c..62c5d62df25 100644 --- a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx @@ -53,8 +53,8 @@ export interface PlansPanelProps { * It is used in the Linode create, Kubernetes and Database create flows. * It contains ample logic to determine which plans are available based on the selected region availability and display related visual indicators: * - If the region is not supported, show an error notice and disable all plans. - * - If more than half plans are disabled, show the limited availability banner and hide the limited availability tooltip - * - If less than half plans are disabled, hide the limited availability banner and show the limited availability tooltip + * - If more than half the plans are disabled, show the limited availability banner and hide the limited availability tooltip + * - If less than half the plans are disabled, hide the limited availability banner and show the limited availability tooltip */ export const PlansPanel = (props: PlansPanelProps) => { const { From ad9e919c3e566f757238e60fca6b1ff43db524db Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Wed, 5 Jun 2024 08:14:16 -0400 Subject: [PATCH 6/6] feedback @mjac0bs --- .../manager/src/features/components/PlansPanel/PlansPanel.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx index 62c5d62df25..432a1670a0c 100644 --- a/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx +++ b/packages/manager/src/features/components/PlansPanel/PlansPanel.tsx @@ -145,7 +145,9 @@ export const PlansPanel = (props: PlansPanelProps) => { <>