-
Notifications
You must be signed in to change notification settings - Fork 364
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
change: [M3-8159] - Modify limited availability banner display logic #10536
Conversation
Coverage Report: ✅ |
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.
- Confirmed the "learn more" link has moved from the notice to the class description helper text above the plans table
- Confirmed that the notice displays if the majority of plans are disabled and tooltip does not; else, the inverse
- Test passing 🧪
Approving, but noting one thing.
For this branch, compared to dev (which is the only place I can see Bare Metal), we're now seeing two notices and the class description copy is not visible. I don't have much familiarity with the Bare Metal plans, so not sure if this change was intended, impactful, or not.
Dev | This Branch |
---|---|
packages/manager/src/features/components/PlansPanel/PlansPanel.tsx
Outdated
Show resolved
Hide resolved
….tsx Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
thanks @mjac0bs - bare metal plans are kind of a special case (and you need an account flag for them to even show up). This is prolly a bit out of scope but i removed the display of the limited availability banner for them anyway since they handle their own. Good catch! |
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.
Looks good.
Description 📝
This PR modifies the display logic of the limited availability banner in the Selection Plan. This is the result of product wanted to limit the display of the banner, especially as it relates to the 512Gb plan since the banner would always display for any class with a 512 plan.
The solution was to only display the banner if more than half of the plans are disabled. Since that logic already existed for the disabled tooltip, I leveraged this util to apply it to the notice.
As a result, the logic is as follow:
Additionally, it was decided to include the "learn more" link in the Class description copy and remove it from the banner so it is always available for the user now that the banner won't display often.
Changes 🔄
Target release date 🗓️
6/10/2024
Preview 📷
How to test 🧪
Verification steps
As an Author I have considered 🤔
Check all that apply