feat: [M3-7789] – Disable 512GB plans #10228

dwiley-akamai · 2024-02-23T21:27:09Z

Description 📝

We are temporarily disabling the 512 GB plans for both Dedicated CPU and Premium CPU plan types. This approach aims to direct users towards accessible offerings and ensure clarity in our plan options. To achieve this, we introduce a "placeholder" 512 GB entry in the Linode plans table. This entry is visually distinct (greyed out).

We are also moving away from the "Sold Out" chip in favor of a tooltip informing the user of limited availability + a dismissible banner with a docs link the user can navigate to in order to learn more about plans and availability.

Changes 🔄

Disable 512 GB Plan Entries: In the Cloud Manager's Linode plans table, the 512 GB plans for Dedicated CPU and Premium CPU types are disabled, making them easily identifiable as not available.
Tooltip/Message Clarification: A tooltip or message is presented when users hover over or engage with the disabled 512 GB plan entry, providing an explanation for its unavailability.
Placeholder Entry Creation: A "placeholder" 512GB plan entry is introduced to the Linode plans table with the specific purpose of being disabled. This entry is designed to be non-selectable and will not initiate any plan.
Feature Flag Management: The activation or deactivation of this disabled option is managed through a feature flag, ensuring flexibility in its implementation.

Target release date 🗓️

03/18/2024

Preview 📷

Edit: 3/14/24 revised styling of dismissible availability banner

Bold text and reduced padding around the notice to better align with existing notices

Linode Create

Dedicated tab:

Premium and GPU tabs:

If Premium/GPU plans are not supported in the selected region...

If Premium/GPU plans are supported in the selected region...

and half or less of the plans are unavailable...
and more than half of the plans are unavailable... (notice the slight change in wording in the dismissible banner, and the lack of help icons in the table)

Tabs in Linode Resize, LKE Create, and LKE "Add a Node Pool" drawer should match what you see for Linode Create (given the same region is selected). The Dedicated tab for Database Create should also match up.

The docs link in the dismissible banner should correspond to the plan tab (Dedicated tab --> Dedicated docs link, etc.).

How to test 🧪

Prerequisites

Pull down branch or preview link

Verification steps for disabled 512 GB plans

verify Linode Create flow for Dedicated, Premium, and GPU plans
- with the feature flag for "Gecko" on, select an Edge region and ensure that the dismissible banner from this PR re: plans and availability is not visible (Edge regions only support up to 128GB plans so there's no reason to display the dismissible banner in that situation)
verify Kubernetes Create flow and "Add a Node Pool" drawer for Dedicated & Premium plans
verify Databases Create flow for Dedicated plans

As an Author I have considered 🤔

…nstead of constants.ts; small introduction for shifting to Limited Availability instead of Sold Out

packages/manager/src/features/components/PlansPanel/utils.ts

cpathipa

Small screen is rendering LA chip instead of Sold out is this expected ?

cpathipa

Tooltip message is not close enough to the sold out chip.

cpathipa

Looks like in Premium sold out chip is missing for 512gb plan in Linode and LKE plans table.
Steps:

Select the region where premium plans are not available.
Navigate to Premium tab and notice sold out chip missing.

jdamore-linode · 2024-02-27T18:19:30Z

Not specific to the changes in this PR, but the pattern of adding a tooltip to the chip in a disabled plan row (or card, on small screens) doesn't work on touch devices, because the chip cannot be interacted with and therefore the tooltip cannot be displayed:

IMG_2667.mp4

Less crucially, it looks like passing a React node, rather than a string, to the Tooltip component causes it to lose its aria-label (and makes it unable to render a value for its data-qa- attribute, which we can figure out a way around):

String:

Node:

I don't think there are any accessibility implications here since as far as I can tell there's no way to actually interact with these disabled plan rows in a way that would cause the ARIA label to be announced, but wanted to call it out in case somebody with more knowledge than me has a better understanding of the implications.

(Edit: still planning to follow up with some test additions soon)

cpathipa

Functionality LGTM!, except for the small screen 'LA' chip issue. I've left a few comments for improvements.

- Verified Linode create flow for Dedicated/Premium CPUs
- Verified Kubernetes create flow for Dedicated/Premium CPUs
- Verified Databases create flow for Dedicated CPUs

cpathipa · 2024-02-27T19:20:30Z

packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlanContainer.tsx

@@ -72,7 +72,7 @@ export const KubernetesPlanContainer = (
          disabled={disabled}
          getTypeCount={getTypeCount}
          idx={id}
-          isPlanSoldOut={disabled ? false : isPlanSoldOut} // no need to add sold out chip if the whole panel is disabled (meaning that the plan isn't available for the selected region)
+          isPlanSoldOut={disabled ? false : isPlanSoldOut} // no need to add "Sold Out" chip if the whole panel is disabled (meaning that the plan isn't available for the selected region)


Any specific reason why we are not showing "Sold out" chip when entire plans panel is disabled ?

There are DCs where plans are not available and that's sufficient from a product perspective. In other words, you can't sell out of plans that aren't available.

packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlanSelection.tsx

packages/manager/src/features/components/PlansPanel/PlanContainer.tsx

jdamore-linode · 2024-02-27T21:33:39Z

@jaalah-akamai Pushed a handful of unit tests to confirm that the 512 GB plan selections get disabled -- there's room for more coverage (confirming other plans don't get disabled, confirming it works for "Premium" 512 GB plans in addition to "Dedicated" and that the tooltip message reflects that, Cypress coverage, etc.) and I may work on that some tomorrow, but hoping this helps out some in the meantime

github-actions · 2024-02-27T21:53:22Z

Coverage Report: ✅
Base Coverage: 81.42%
Current Coverage: 81.42%

carrillo-erik · 2024-02-28T17:34:44Z

The Sold Out chip is hard to see in Light Mode.
I am also experiencing the issue mentioned by @cpathipa in which the chip switches from Sold Out to LA when going from Desktop view to Mobile view.
I also ran into the issue @jdamore-linode identified where the chip component does not show the tooltip content in mobile view because the user cannot interact with it.

jaalah-akamai · 2024-02-28T22:16:01Z

@carrillo-erik @cpathipa Unfortunately we had some shifting product decisions today that will solve or make most of these irrelevant. I'm going to move this back into WIP, and any reviews that are still relevant I'll tag you all in.

In addition, disabled states don't need to meet color contrast ratios... but it doesn't mean we shouldn't address it. We can tackle that in a separate ticket if need be.

@jdamore-linode Good call, I have a solution in mind for that.

dwiley-akamai · 2024-03-12T20:46:15Z

The most recent commit brings us to a point of parity when the feature flag is off, preventing a scenario where all plans on a tab are disabled with no explanation:

Prod (GPU tab)	This branch (GPU tab, `disableLargestGbPlans` flag off)

jdamore-linode · 2024-03-12T21:01:31Z

I'm still having a hard time with the tooltip on touch devices -- I can get it to appear now, but I can't get it to appear consistently. And when it does appear, it sometimes stays indefinitely, and other times seemingly dismisses itself after a second or so:

IMG_2730.mp4

dwiley-akamai · 2024-03-13T19:28:40Z

I'm still having a hard time with the tooltip on touch devices -- I can get it to appear now, but I can't get it to appear consistently. And when it does appear, it sometimes stays indefinitely, and other times seemingly dismisses itself after a second or so:

Thanks for checking that @jdamore-linode. Is this something you're only seeing in this section of the app or is it a more general issue with tooltips in the app?

I'm also seeing that account/user-profile.spec.ts is failing, looking into that but do you happen to have any insights on it?

jdamore-linode · 2024-03-13T20:09:54Z

Is this something you're only seeing in this section of the app or is it a more general issue with tooltips in the app?

@dwiley-akamai Other tooltips appear to be working fine*, it looks like it's specific to the tooltip inside the 512 GB plan row.

*(I am seeing a little bit of weirdness with other tooltips where sometimes tapping the icon reveals the tooltip without the icon turning blue, but I'm seeing that in Prod too so definitely not related to this PR)

I'm also seeing that account/user-profile.spec.ts is failing, looking into that but do you happen to have any insights on it?

It seems as though the data-qa-tooltip attribute is not being applied to tooltips so Cypress is failing to find it:

This seems to be the cause for most of the test failures, although the lke-update.spec.ts failure might be something else (at a glance it looks like only the 512 GB plan is getting rendered in the "Add a Node Pool" drawer) and the update-firewall.spec.ts failure looks like it might be a fluke but I'll look more closely if it fails again.

carrillo-erik · 2024-03-13T21:06:11Z

I'm experiencing an issue with the tooltip in mobile view. The tooltip text is not showing in the create Linode flow or the create LKE flow. I tried to activate the tooltip and I can see it change colors upon clicking it, however, the text does not display. Please look at screen recording below.

Screen.Recording.2024-03-13.at.2.02.07.PM.mov

carrillo-erik · 2024-03-13T21:44:07Z

packages/manager/src/features/components/PlansPanel/constants.ts

+import type { PlanSelectionType } from './types';
+import type { ExtendedType } from 'src/utilities/extendType';
+
+export const LIMITED_AVAILABILITY_TEXT = 'This plan has limited availability.';


Don't we usually have a delimiter on these type of strings? I think this should be This-plan-has-limited-availability.

I don't believe so, is there an example in the codebase?

Actually, I confused this with how we set the data-testid strings. Those we use a delimiter.

….spec.ts

bnussman-akamai · 2024-03-14T13:24:21Z

packages/manager/src/components/Tooltip.tsx

+interface TooltipDataAttribute extends TooltipProps {
+  ['data-qa-tooltip']: string | undefined;
+  title: React.ReactNode;
+}
+
+interface TooltipNonDataAttribute extends TooltipProps {
+  title: string;
+}
+
+// Discriminated union to enforce `data-qa-tooltip` if title is a ReactNode
+export type EnhancedTooltipProps =
+  | TooltipDataAttribute
+  | TooltipNonDataAttribute;
+


Do we really need all of this?

In my opinion, I don't think we should be enforcing that Toolips have data-qa-tooltip under any circumstance.

Agree -- I get how this came about, but the data-qa-* and data-testid attributes should be inconsequential to the app itself (i.e. in an ideal world we should be able to remove all of these attributes throughout the app without it causing any build issues or changes in behavior to the app itself)

This may break some tests. However title should almost always be a string now since we determined ReactNode (which was used for links) was inaccessible.

@jaalah-akamai Locally I undid these and the related dataAttrs changes throughout the diff, the currently failing E2E tests pass aside from lke-update.spec.ts (possibly unrelated) and there are no TS errors. Would it be objectionable to reintroduce the discriminated union and associated changes as part of a separate ticket?

edit: as agreed upon in Standup, I'll undo these changes and create a separate ticket for looking into reintroducing them

edit 2: created M3-7882

bnussman-akamai · 2024-03-14T13:25:29Z

packages/manager/src/features/Account/Maintenance/MaintenanceTableRow.tsx

@@ -72,7 +72,7 @@ export const MaintenanceTableRow = (props: AccountMaintenance) => {
      <Hidden lgDown>
        <TableCell>
          {isTruncated ? (
-            <Tooltip title={<HighlightedMarkdown textOrMarkdown={reason} />}>
+            <Tooltip title={reason}>


Should this be reverted?

abailly-akamai · 2024-03-14T13:36:12Z

@dwiley-akamai we also need to fix the treatment of this banner, it's inconsistent with others:

should be bolded text
shouldn't have top and bottom padding

New banner

Existing banners

dwiley-akamai · 2024-03-14T14:14:24Z

@dwiley-akamai we also need to fix the treatment of this banner, it's inconsistent with others:

should be bolded text

shouldn't have top and bottom padding

New banner

Existing banners

We're matching the approved UX wireframes on this, but I will double-check with Lauren on these two points

edit: confirmed that this banner should match the existing ones, commit with changes forthcoming

…tices

carrillo-erik · 2024-03-14T17:00:30Z

It seems as though the data-qa-tooltip attribute is not being applied to tooltips so Cypress is failing to find it:
This seems to be the cause for most of the test failures, although the `lke-update.spec.ts` failure might be something else (at a glance it looks like _only_ the 512 GB plan is getting rendered in the "Add a Node Pool" drawer) and the `update-firewall.spec.ts` failure looks like it might be a fluke but I'll look more closely if it fails again.

@jdamore-linode
I wonder if this has to do with how the string for data-qa-tooltip is being set. My understanding is that data-* attributes values should consist of delimited strings which is consistent to how we set values for data-testid. It could be that it's reading each word in the string as a separate value for the data-qa-tooltip. Will need further research on this matter.

…ughout the diff

dwiley-akamai · 2024-03-14T21:55:46Z

packages/manager/cypress/e2e/core/kubernetes/lke-update.spec.ts

Per the video of the run, the test was failing because the Dedicated tab consisted of just the disabled 512 GB plan, so this code brings the test to the Shared CPU tab instead so it could proceed as it typically does.

Ok, I was just running the test locally and noticed that as well. It would get stuck trying to click the + symbol but it was disabled since the 512GB plan is not available.

bnussman-akamai · 2024-03-18T12:58:38Z

Functionality is looking good so far. Going to keep reviewing!

UX feedback: I think the Learn more about plans and availability. dismissible banner is kind of extra. I feel like that should just be included as normal copy. I don't think we should be warning our users to "Learn more"

jaalah-akamai · 2024-03-18T13:07:01Z

Functionality is looking good so far. Going to keep reviewing!

UX feedback: I think the Learn more about plans and availability. dismissible banner is kind of extra. I feel like that should just be included as normal copy. I don't think we should be warning our users to "Learn more"

This is something the team is aware of and will follow up in a future release if they feel it needs to be changed - thanks for the feedback though

abailly-akamai

Approving pending as a follow up

improvement of the tooltip accessibility/behavior for cards
Improved coverage (e2e & new units)
naming conventions (mostClassPlansAreLimitedAvailability, isMajorityLimitedAvailabilityPlans types & _types, plans & _plans, plansForThisLinodeTypeClass & _plansForThisLinodeTypeClass etc)

Also the reasoning behind making the banner dismissible is dubious, not to mention inconsistent with other banners in the plan panel

DevDW added 3 commits February 22, 2024 12:20

Add feature flag for disabling 512 GB plans

3e82ff2

Desired state of affairs for Linodes and LKE

72ceb24

Working for DBaaS as well but there is a TS error and cleanup necessary

e4a475c

dwiley-akamai self-assigned this Feb 23, 2024

Move generateUnavailableMessageFor512GbPlans into PlanSelection.tsx i…

8acbdf3

…nstead of constants.ts; small introduction for shifting to Limited Availability instead of Sold Out

dwiley-akamai commented Feb 23, 2024

View reviewed changes

packages/manager/src/features/components/PlansPanel/utils.ts Outdated Show resolved Hide resolved

jaalah-akamai self-assigned this Feb 27, 2024

Cleanup and fix type issues

a3d08be

jaalah-akamai added Ready for Review 🚨 Urgent labels Feb 27, 2024

Added changeset: Disable 512GB Plans

f34b765

jdamore-linode marked this pull request as ready for review February 27, 2024 17:01

jdamore-linode requested a review from a team as a code owner February 27, 2024 17:01

jdamore-linode requested review from carrillo-erik and cpathipa and removed request for a team February 27, 2024 17:01

cpathipa reviewed Feb 27, 2024

View reviewed changes

jdamore-linode added 2 commits February 27, 2024 15:44

Move 'describe' to top level

b8837b9

Add unit tests for disabled 512 GB plan selection

0cf179e

jaalah-akamai added Requires Changes and removed Ready for Review labels Feb 27, 2024

jaalah-akamai removed the 🚨 Urgent label Feb 28, 2024

dwiley-akamai requested review from cpathipa and jaalah-akamai March 13, 2024 19:19

carrillo-erik reviewed Mar 13, 2024

View reviewed changes

passing user-profile.spec.ts, attempt to get passing display-settings…

bbdc5c2

….spec.ts

bnussman-akamai reviewed Mar 14, 2024

View reviewed changes

Revise styling of dismissible availability banner to match similar no…

ab8f24f

…tices

DevDW added 2 commits March 14, 2024 14:21

Undo discriminated union pattern for Tooltip and related changes thro…

b2825a7

…ughout the diff

passing lke-update.spec.ts

6cb6207

dwiley-akamai requested a review from a team as a code owner March 14, 2024 21:51

dwiley-akamai requested review from jdamore-linode and removed request for a team March 14, 2024 21:51

Remove stray console.log()

5a6b2ad

dwiley-akamai commented Mar 14, 2024

View reviewed changes

jaalah-akamai approved these changes Mar 18, 2024

View reviewed changes

bnussman-akamai approved these changes Mar 18, 2024

View reviewed changes

abailly-akamai approved these changes Mar 18, 2024

View reviewed changes

jaalah-akamai merged commit fded40a into linode:develop Mar 18, 2024
18 checks passed

This was referenced Mar 18, 2024

Release v1.115.0 - release → staging #10292

Merged

Release v1.115.0 - staging → master #10293

Merged

dwiley-akamai mentioned this pull request Apr 22, 2024

change: [M3-7998] – "Limited Availability" --> "Limited Deployment Availability" phrasing change #10394

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [M3-7789] – Disable 512GB plans #10228

feat: [M3-7789] – Disable 512GB plans #10228

dwiley-akamai commented Feb 23, 2024 •

edited

cpathipa left a comment

cpathipa left a comment

cpathipa left a comment •

edited

jdamore-linode commented Feb 27, 2024 •

edited

cpathipa left a comment

cpathipa Feb 27, 2024

jaalah-akamai Feb 28, 2024

jdamore-linode commented Feb 27, 2024

github-actions bot commented Feb 27, 2024 •

edited

carrillo-erik commented Feb 28, 2024

jaalah-akamai commented Feb 28, 2024 •

edited

dwiley-akamai commented Mar 12, 2024 •

edited

jdamore-linode commented Mar 12, 2024

dwiley-akamai commented Mar 13, 2024

jdamore-linode commented Mar 13, 2024 •

edited

carrillo-erik commented Mar 13, 2024

carrillo-erik Mar 13, 2024

dwiley-akamai Mar 14, 2024

carrillo-erik Mar 14, 2024

bnussman-akamai Mar 14, 2024

jdamore-linode Mar 14, 2024 •

edited

jaalah-akamai Mar 14, 2024

dwiley-akamai Mar 14, 2024 •

edited

bnussman-akamai Mar 14, 2024

abailly-akamai commented Mar 14, 2024

dwiley-akamai commented Mar 14, 2024 •

edited

carrillo-erik commented Mar 14, 2024

dwiley-akamai Mar 14, 2024

carrillo-erik Mar 14, 2024

bnussman-akamai commented Mar 18, 2024

jaalah-akamai commented Mar 18, 2024

abailly-akamai left a comment •

edited

feat: [M3-7789] – Disable 512GB plans #10228

feat: [M3-7789] – Disable 512GB plans #10228

Conversation

dwiley-akamai commented Feb 23, 2024 • edited

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Prerequisites

Verification steps for disabled 512 GB plans

As an Author I have considered 🤔

cpathipa left a comment

Choose a reason for hiding this comment

cpathipa left a comment

Choose a reason for hiding this comment

cpathipa left a comment • edited

Choose a reason for hiding this comment

jdamore-linode commented Feb 27, 2024 • edited

cpathipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamore-linode commented Feb 27, 2024

github-actions bot commented Feb 27, 2024 • edited

carrillo-erik commented Feb 28, 2024

jaalah-akamai commented Feb 28, 2024 • edited

dwiley-akamai commented Mar 12, 2024 • edited

jdamore-linode commented Mar 12, 2024

dwiley-akamai commented Mar 13, 2024

jdamore-linode commented Mar 13, 2024 • edited

carrillo-erik commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamore-linode Mar 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dwiley-akamai Mar 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai commented Mar 14, 2024

dwiley-akamai commented Mar 14, 2024 • edited

carrillo-erik commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai commented Mar 18, 2024

jaalah-akamai commented Mar 18, 2024

abailly-akamai left a comment • edited

Choose a reason for hiding this comment

dwiley-akamai commented Feb 23, 2024 •

edited

cpathipa left a comment •

edited

jdamore-linode commented Feb 27, 2024 •

edited

github-actions bot commented Feb 27, 2024 •

edited

jaalah-akamai commented Feb 28, 2024 •

edited

dwiley-akamai commented Mar 12, 2024 •

edited

jdamore-linode commented Mar 13, 2024 •

edited

jdamore-linode Mar 14, 2024 •

edited

dwiley-akamai Mar 14, 2024 •

edited

dwiley-akamai commented Mar 14, 2024 •

edited

abailly-akamai left a comment •

edited