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-7580] - Add list view for Linode Clone/Backup #10182

Merged

Conversation

hkhalil-akamai
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai commented Feb 12, 2024

Description πŸ“

Introduces a list view for selecting linodes to clone/restore from in the create UI.

This is the final PR of the Linode Clone UI Updates.

Changes πŸ”„

  • Introduces a new list view for large-screen (mdUp) devices
  • List view displays Linode label, status, image, plan and region
  • Includes inline "Power Off" action button for running linodes.

Preview πŸ“·

Before After
Clone Linode
Screenshot 2024-02-13 at 4 28 38 PM Screenshot 2024-02-13 at 4 28 02 PM
Backups
Screenshot 2024-02-13 at 4 28 47 PM Screenshot 2024-02-13 at 4 28 23 PM

How to test πŸ§ͺ

Prerequisites

  • Ensure "Linode Clone UI Changes" feature flag is turned on

Verification steps

  • Enter the Linode Create flow, either through the "Create" menu on the top-left corner of the app or by selecting "Clone" in the action menu for a linode
  • In both the "Clone Linode" and "Backups" tabs:
    • Linode label, status, image, plan and region are displayed
    • Sorting works as expected for all fields
    • For running Linodes: Power Off button is displayed and clicking it displays a confirmation dialog
    • For small displays, list reverts back to the original cards view
  • In the "Clone Linode" tab only:
    • Selecting a linode populates the region, plan selection and label fields
  • In the "Backups" tab only:
    • Selecting a linode displays all available backups for that linode

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

@hkhalil-akamai hkhalil-akamai self-assigned this Feb 12, 2024
@hkhalil-akamai hkhalil-akamai changed the title upcoming: [M3-7580] - Add list view in Linode Create from Clone and Backup flows upcoming: [M3-7580] - Add list view for Linode Clone/Backup Feb 13, 2024
@hkhalil-akamai hkhalil-akamai added Linode Clone UI Updates Linodes Dealing with the Linodes section of the app labels Feb 13, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to display the "Power Off" button in the Backups tab? It makes sense in Clone Linode due to the new recommendation to power off before cloning, but this doesn't apply when creating from a backup.

@hkhalil-akamai
Copy link
Contributor Author

@hkhalil-akamai you also need to trigger the code coverage and e2e suite with another commit. I wonder if the e2e suite is going to trip or keep testing the old UI (if it is covered at all, which I think it is) - either way you will need to mock the flag to test the new UI

Ooh I didn't think about this, does the test environment use development flags? (I believe the flag is enabled in dev).

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Thx for the changes! confirming code and UI, pending:

  • improved coverage
  • confirming stacked notices with UX
  • resolution of e2e

(for instance this is how they did it for Placement Group)
Screenshot 2024-02-14 at 15 24 15

@abailly-akamai
Copy link
Contributor

abailly-akamai commented Feb 14, 2024

@hkhalil-akamai

does the test environment use development flags

you can mock those. ex: https://github.com/linode/manager/blob/develop/packages/manager/cypress/e2e/core/account/account-login-history.spec.ts#L44-L48

You can mock the flag off for now so it passes for this PR, but you need a follow up ticket to handle the new flow before the flag is turned on in any environment (if you get failures)

Copy link

github-actions bot commented Feb 15, 2024

Coverage Report: βœ…
Base Coverage: 81.32%
Current Coverage: 81.34%

@hkhalil-akamai hkhalil-akamai added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Feb 20, 2024
@hkhalil-akamai
Copy link
Contributor Author

Changed notices to use bullet points as suggested by @abailly-akamai:

Before After
Clone Linode
Screenshot 2024-02-21 at 6 32 36 PM Screenshot 2024-02-21 at 6 30 34 PM
Backups
Screenshot 2024-02-21 at 6 32 41 PM Screenshot 2024-02-21 at 6 30 21 PM

@hkhalil-akamai
Copy link
Contributor Author

Verified all E2E tests pass locally with feature flag enabled.

Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

This looks great! Nice work - As I mentioned before, I would love to get some skeleton loading states for the table rows. Otherwise let's ship it πŸš€

@hkhalil-akamai hkhalil-akamai merged commit d9b5b92 into linode:develop Feb 26, 2024
18 checks passed
@hkhalil-akamai hkhalil-akamai deleted the M3-7580-linode-clone-ui-updates branch February 26, 2024 21:12
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! Linodes Dealing with the Linodes section of the app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants