Skip to content

fix: [M3-9465] - Make LKE/LKE-E spacing tweaks - #11827

Merged
mjac0bs merged 7 commits into
linode:developfrom
mjac0bs:M3-9465-spacing-tweaks
Mar 13, 2025
Merged

mjac0bs merged 7 commits into
linode:developfrom
mjac0bs:M3-9465-spacing-tweaks

Conversation

@mjac0bs

@mjac0bs mjac0bs commented Mar 11, 2025

Copy link
Copy Markdown
Contributor

Description 📝

This PR makes some minor spacing changes to improve some inconsistencies throughout LKE as requested by UX during review of LKE-E.

Changes 🔄

  • Update the Cluster Tier selection card padding to 16px all around and the space between the card heading and first subheading to 4px
  • Update the spacing at the bottom of the NodeTable to be equal on top and bottom (8px)
  • Decrease the spacing at the top of the Labels & Taints drawer to be more standard (24px) and increase the spacing between the tables and the 'Add' buttons (12px)

Target release date 🗓️

3/25

Preview 📷

Before After
CardPaddingOld CardPaddingNew
CardInnerMarginOld CardInnerMarginNew
NodeTableOld NodeTableFixed
LTCopyOld LTCopyNew
LabelTaintTableOld LabelTaintTableNew

How to test 🧪

Prerequisites

(How to setup test environment)

  • Have the LKE-E customer tag on your account (see project tracker) and have the feature flag enabled

Reproduction steps

(How to reproduce the issue, if applicable)

  • Check out dev and see the current spacing inconsistencies

Verification steps

(How to verify changes)

  • Confirm spacing looks more consistent with the changes above
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

@mjac0bs mjac0bs added UX/UI Changes for UI/UX to review LKE Related to Linode Kubernetes Engine offerings LKE-Enterprise labels Mar 11, 2025
@mjac0bs mjac0bs self-assigned this Mar 11, 2025
@mjac0bs mjac0bs changed the title M3 9465 spacing tweaks fix: [M3-9465] - Make LKE/LKE-E spacing tweaks Mar 11, 2025
@mjac0bs
mjac0bs marked this pull request as ready for review March 11, 2025 20:19
@mjac0bs
mjac0bs requested a review from a team as a code owner March 11, 2025 20:19
@mjac0bs
mjac0bs requested review from cliu-akamai and hkhalil-akamai and removed request for a team March 11, 2025 20:19
@mjac0bs
mjac0bs force-pushed the M3-9465-spacing-tweaks branch from e4ef12d to 18a5bb1 Compare March 11, 2025 20:20
heading="LKE"
onClick={() => handleClusterTierSelection('standard')}
subheadings={[StandardSubheadings]}
sxCardBase={{ padding: '16px' }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We made this change as a one-off for these particular cards to give the text-heavy card more breathing room.

If we want to make this change to the base component styles, we could make that change instead. Worth noting: selection card styles were already recently updated to the 12px padding they have all around here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tbh 16px feels like a bit too much padding but I will defer to the team's decision (maybe a cafe topic?)

Another place that's text heavy is the mobile Linode Plans
image

@mjac0bs mjac0bs Mar 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also think that 16px seems a bit much, but deferred to UX for the cluster tier cards. I'll add the topic as a (low priority) cafe topic to see if there's consensus among the wider group.

One thing that might consistently help the text-heaviness more than base card padding is some padding between subheading list items.

@mjac0bs
mjac0bs requested a review from hana-akamai March 11, 2025 20:33
@github-actions

github-actions Bot commented Mar 11, 2025

Copy link
Copy Markdown

Coverage Report:
Base Coverage: 79.83%
Current Coverage: 79.83%

@linode-gh-bot

Copy link
Copy Markdown

Cloud Manager UI test results

🔺 1 failing test on test run #4 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing537 Passing3 Skipped129m 30s

Details

Failing Tests
SpecTest
linode-config.spec.tsEnd-to-End » Clones a config

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/linodes/linode-config.spec.ts"

@hkhalil-akamai hkhalil-akamai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Verified expected spacing around selection card.
✅ Verified expected spacing in labels and taints drawer.
✅ Found no regressions in the LKE cluster create flow and Labels and Taints drawer.

Thanks Mariah! 🚀

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Mar 12, 2025
@github-project-automation github-project-automation Bot moved this from Review to Approved in Cloud Manager Mar 12, 2025
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Mar 12, 2025
@mjac0bs
mjac0bs merged commit de43803 into linode:develop Mar 13, 2025
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Cloud Manager Mar 13, 2025
@cypress

cypress Bot commented Mar 13, 2025

Copy link
Copy Markdown

Cloud Manager E2E    Run #7395

Run Properties:  status check passed Passed #7395  •  git commit de438033cc: fix: [M3-9465] - Make LKE/LKE-E spacing tweaks (#11827)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #7395
Run duration 32m 28s
Commit git commit de438033cc: fix: [M3-9465] - Make LKE/LKE-E spacing tweaks (#11827)
Committer Mariah Jacobs
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 3
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 539
View all changes introduced in this branch ↗︎

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! LKE Related to Linode Kubernetes Engine offerings LKE-Enterprise

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants