feat: [M3-8855] - Surface Node Pool Tags #11368
Conversation
hana-akamai
requested review from
cliu-akamai,
carrillo-erik,
harsh-akamai and
mjac0bs
and removed request for
a team
|
Coverage Report: ✅ |
| <Box | ||
| alignItems="center" | ||
| data-testid={encryptionStatusTestId} | ||
| display="flex" | ||
| > | ||
| <Typography>Pool ID {poolId}</Typography> | ||
| <StyledVerticalDivider /> | ||
| <EncryptedStatus | ||
| encryptionStatus={encryptionStatus} | ||
| tooltipText={DISK_ENCRYPTION_NODE_POOL_GUIDANCE_COPY} | ||
| /> | ||
| </Box> | ||
| ) : ( | ||
| <Typography>Pool ID {poolId}</Typography> | ||
| )} | ||
| </Box> |
There was a problem hiding this comment.
mobile.mov
We'll want to consider mobile view here, and might need some UX input. (Should we still have the ability to see tags in this view?) Currently, the tag cell will overlap with the pool id/encryption box once we get to ~500px in the viewport, and it gets worse at smaller screens (iPhone SE). Adjusting the box's width of 100% to less helps, but the tooltip still gets hidden. Maybe displaying tags under the pool id and encryption status is most readable?
Looks like there is also an existing issue where the Unlock icon has a min width of 16px, but the Lock icon does not, leading it to shrink and virtually disappear (and leave some odd spacing) behind at the smaller screen sizes. We could fix that here.
There was a problem hiding this comment.
@mjac0bs Good catches, should be addressed now. I moved the tags to a separate row on mobile and removed the minWidth of 16px
There was a problem hiding this comment.
Looks better! I think we should actually do this for tablet screen sizes too. There's an edge case with really long tags where, between tablet and mobile breakpoints, we still get some overlapping. Switching to displaying the tags below at the md breakpoint is also consistent with the cluster's tags. (Which seem to have their own buggy display issue... I'll create a follow up ticket to fix cluster tags. Update: M3-9009)
No issues after using md breakpoints instead of sm:
Screen.Recording.2024-12-11.at.12.59.02.PM.mov
Cloud Manager UI test results🎉 466 passing tests on test run #13 ↗︎
|
harsh-akamai
added
Approved
Cloud Manager E2E
|
||||||||||||||||||||||||||||
| Project |
Cloud Manager E2E
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 29m 10s |
| Commit |
|
| Committer | Hana Xu |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
3
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
469
|
| View all changes introduced in this branch ↗︎ | |
* surface tags for node pools * wip e2e * fix e2e * Added changeset: Surface Node Pool Tags * fix unit test * Added changeset: tags to KubeNodePoolResponse and CreateNodePoolData * clean up * address feedback * clean up * fix tags cutting off in mobile * spacing * use md breakpoint instead of sm * styling
Description 📝
The /pools endpoint returns Tags for node pools, but we don't currently surface them in the UI: https://techdocs.akamai.com/linode-api/reference/get-lke-node-pool
At the bottom of each node pool section, we will now display the list of tags associated with a node pool and the option to add a new tag.
Changes 🔄
Preview 📷
Screen.Recording.2024-12-05.at.12.46.32.PM.mov
How to test 🧪
Verification steps
(How to verify changes)
Add a Tagbutton at the bottom of a Node Pool tableAuthor 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
As an Author, before moving this PR from Draft to Open, I confirmed ✅