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

upcoming: [M3-7868] - Gecko Beta Demo feedback #10284

Merged

Conversation

hana-akamai
Copy link
Contributor

Description 📝

Address feedback from the Gecko Beta Demo/Review meeting

Changes 🔄

  • Warning notice updates
    • Add period to end of sentences
    • Replace "standard pricing" warning text from the edge plan table to "billing"
    • Add warning text to the caution notice for edge migrations
  • Storage table column should display as normal (should not be N/A) for edge plans
  • Filter edge regions from Image Upload

Preview 📷

Before After
image image
image image
image image

How to test 🧪

Prerequisites

  • Ensure your account has the correct customer tag (reach out for more info)

Verification steps

  • Go to /linodes/create and select an Edge region
    • The edge plan table warning notice should say "billing" instead of "standard pricing"
    • The Storage column should display the storage amount instead of N/A
  • On the Linode landings table, open the action menu for a Linode located in an edge region and click on "Migrate"
    • There should be a warning bullet about edge migrations in the Caution notice
  • Go to /images/create/upload and open the region select dropdown
    • There should be no edge regions displayed

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@hana-akamai hana-akamai added the Gecko Beta Relating to Gecko project label Mar 14, 2024
@hana-akamai hana-akamai self-assigned this Mar 14, 2024
@hana-akamai hana-akamai marked this pull request as ready for review March 14, 2024 16:52
@hana-akamai hana-akamai requested a review from a team as a code owner March 14, 2024 16:52
@hana-akamai hana-akamai requested review from cpathipa and abailly-akamai and removed request for a team March 14, 2024 16:52
@jaalah-akamai jaalah-akamai self-requested a review March 14, 2024 18:16
@mjac0bs mjac0bs self-requested a review March 14, 2024 19:56
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

All the described changes look good on my account with the customer tag! 🚢

I'm not sure what kind of test coverage you've got for Gecko, and whether you want to consider adding coverage for any of the warnings.

Also not specific to this PR: Our migrate warning list is so long. 😅 I wonder if customers actually read that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor and not directly related to this PR, but probably worth a backlog ticket: our filled state for disabled checkboxes is not great in dark mode. We can't really see it. It could be nice to have the disabled mouse icon over those checkboxes too.

Screenshot 2024-03-14 at 1 13 24 PM
Screenshot 2024-03-14 at 1 13 35 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, created M3-7884 to address this

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Mar 14, 2024
Comment on lines +153 to +156
const edgeRegionWarning =
flags.gecko && linodeIsInEdgeRegion
? 'Edge sites may only be migrated to other Edge sites.'
: undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Optional: should this logic live in CautionNotice?

@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Mar 14, 2024
Copy link

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

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

  • Go to /linodes/create and select an Edge region
    • The edge plan table warning notice should say "billing" instead of "standard pricing" ✅
    • The Storage column should display the storage amount instead of N/A ✅
  • On the Linode landings table, open the action menu for a Linode located in an edge region and click on "Migrate"
    • There should be a warning bullet about edge migrations in the Caution notice ✅
  • Go to /images/create/upload and open the region select dropdown
    • There should be no edge regions displayed ✅
  • Periods added for Edge-related warnings ✅

@hana-akamai hana-akamai merged commit b73cc08 into linode:develop Mar 18, 2024
18 checks passed
@hana-akamai hana-akamai deleted the M3-7868-gecko-beta-demo-feedback branch March 18, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Gecko Beta Relating to Gecko project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants