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

feat: [M3-9231] - Improve Node Pool Collapsing UX #11619

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

hana-akamai
Copy link
Contributor

@hana-akamai hana-akamai commented Feb 5, 2025

Description 📝

Improve UX of collapsible Node Pools

Changes 🔄

  • Add default logic for expanded/collapsed node pools
  • Added a Collapse/Expand all pools button
  • Preserve user state to local storage

Preview 📷

Preview
Nodes Pools Cluster view
1
1.node.pool.mov
3
Screen.Recording.2025-02-11.at.12.29.18.PM.mov
4
4.node.pools.mov

Expand/Collapse

Screen.Recording.2025-02-10.at.12.15.32.PM.mov

How to test 🧪

Prerequisites

(How to setup test environment)

  • Have a LKE or LKE-E cluster with multiple node pools with varying amount of nodes in each node pool

Verification steps

(How to verify changes)

  • Test various node pool sizes and nodes in each pool
    • If there's only one node pool, the pool is expanded by default regardless of how many nodes there are
    • if the user has between 1-3 node pools, if the node pool has <10 nodes, keep that pool expanded by default
    • If there are more than 3 node pools, keep them all collapsed by default
  • State should be saved to local storage if the user expands/collapse the node pool and this should override default behavior
  • Test expand/collapse all button
yarn cy:run -s "cypress/e2e/core/kubernetes/lke-update.spec.ts"

You can use the legacy MSW and update the get function in line 836 of serverHandlers.ts to something like:

return HttpResponse.json(
  makeResourcePage([
    nodePoolFactory.build({
      nodes: kubeLinodeFactory.buildList(50),
      count: 50,
    }),
  ])
);
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 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


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@hana-akamai hana-akamai added UX/UI Changes for UI/UX to review LKE Related to Linode Kubernetes Engine offerings labels Feb 5, 2025
@hana-akamai hana-akamai self-assigned this Feb 5, 2025
@hana-akamai hana-akamai force-pushed the M3-9231-improve-node-pool-collapsing-ux branch from d9e8184 to 3abecdd Compare February 6, 2025 21:15
@hana-akamai hana-akamai marked this pull request as ready for review February 10, 2025 17:21
@hana-akamai hana-akamai requested review from a team as code owners February 10, 2025 17:21
@hana-akamai hana-akamai requested review from dmcintyr-akamai, mjac0bs and bnussman-akamai and removed request for a team February 10, 2025 17:21
Copy link

github-actions bot commented Feb 10, 2025

Coverage Report:
Base Coverage: 80.13%
Current Coverage: 80.08%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

My preference would probably be to use an outlined button but I guess either is fine

Screenshot 2025-02-10 at 2 31 00 PM

Screenshot 2025-02-10 at 2 30 41 PM

@hana-akamai
Copy link
Contributor Author

@bnussman-akamai I initially had it outlined, but Daniel didn't want the Collapse/Expand action to have the same order of significance as the 2 other Node Pool actions (Recycle All Node Pools & Add A Node Pool)

@hana-akamai hana-akamai changed the title feat: [M3-9231] - Improve Node Pool Collapsing feat: [M3-9231] - Improve Node Pool Collapsing UX Feb 10, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 501 passing tests on test run #8 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing501 Passing2 Skipped102m 21s

@mjac0bs
Copy link
Contributor

mjac0bs commented Feb 10, 2025

@bnussman-akamai I initially had it outlined, but Daniel didn't want the Collapse/Expand action to have the same order of significance as the 2 other Node Pool actions (Recycle All Node Pools & Add A Node Pool)

@hana-akamai I see where he's coming from and am okay with non-outlined because of it, but I do think we should switch the order of the status filter and the collapse/expand button so that all the buttons are at least grouped together, as Banks had in his suggested screenshot.

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.

Looks good now with those changes!

The only other thing I'm wondering about is this row of CTAs as a whole, especially in mobile view. I do think we have a ticket for adjusting the spacing here (update: it's M3-9061; just updated the AC to include improving mobile view). Wondering if we should use it to reevaluate how/if all these are displayed on small screens.

Screen.Recording.2025-02-11.at.9.28.53.AM.mov

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add'tl Approval Needed Waiting on another approval! LKE Related to Linode Kubernetes Engine offerings UX/UI Changes for UI/UX to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants