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-8215] - Hide monthly network transfer section for distributed regions #10714

Conversation

hana-linode
Copy link
Contributor

@hana-linode hana-linode commented Jul 25, 2024

Description 📝

Distributed compute instances have no transfer pool, so the Monthly Network Transfer section is irrelevant. This PR hides that section and expands the Network Transfer History chart to occupy the space.

Changes 🔄

List any change relevant to the reviewer.

  • Hide the monthly network transfer section for Linodes in a distributed region

Preview 📷

Before After
image image

How to test 🧪

Prerequisites

(How to setup test environment)

  • Ensure your account has the new-dc-testing, edge_testing and edge_compute customer tags
  • Create a Linode in a distributed region if you don't already have one

Verification steps

(How to verify changes)

  • Go to a Linode's details page and then navigate to the Network tab
  • The Monthly Network Transfer section should not display for Linodes in a distributed region
  • The Monthly Network Transfer section should display as normal for Linodes in a core region

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-linode hana-linode added the Gecko LA Relating to Gecko LA label Jul 25, 2024
@hana-linode hana-linode self-assigned this Jul 25, 2024
@hana-linode hana-linode marked this pull request as ready for review July 25, 2024 16:54
@hana-linode hana-linode requested a review from a team as a code owner July 25, 2024 16:54
@hana-linode hana-linode requested review from jdamore-linode and cpathipa and removed request for a team July 25, 2024 16:54
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.

❔ Question:

Are there any plans to update the MNTP dialog to explain that distributed regions won't be reflected there?

Screenshot 2024-07-25 at 1 23 08 PM

@hana-linode
Copy link
Contributor Author

@mjac0bs Good catch, I asked Kendall and he said there was some initial discussion to completely redesign the network transfer UI

Copy link

github-actions bot commented Jul 26, 2024

Coverage Report:
Base Coverage: 82.46%
Current Coverage: 82.47%

@mjac0bs mjac0bs changed the title upcoming: [M3-8215] - Hide monthly network transfer section upcoming: [M3-8215] - Hide monthly network transfer section for distributed regions Jul 26, 2024
@mjac0bs
Copy link
Contributor

mjac0bs commented Jul 26, 2024

@mjac0bs Good catch, I asked Kendall and he said there was some initial discussion to completely redesign the network transfer UI

A complete redesign sounds like it may take some time to actually get designed and implemented - though I'm in agreement that it should happen; I know customer support gets questions about usage and things not being clear. But in the meantime, what do we think about saying "your devices' core regions" or something like that in the tooltip?

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.

🚀

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jul 26, 2024
Comment on lines +27 to +28
const hideNetworkTransfer =
isGeckoGAEnabled && linode.site_type === 'distributed';
Copy link
Member

Choose a reason for hiding this comment

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

Could we simplify this to

Suggested change
const hideNetworkTransfer =
isGeckoGAEnabled && linode.site_type === 'distributed';
const showNetworkTransfer = linode.site_type === 'core';

So that the ternary below could be replaced with a &&

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Jul 29, 2024
@hana-linode hana-linode merged commit 978ddab into linode:develop Jul 29, 2024
19 checks passed
@hana-linode hana-linode deleted the M3-8215-hide-monthly-network-transfer-section branch July 29, 2024 15:44
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 LA Relating to Gecko LA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants