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

chore: [M3-9440] - Improve Banner Spacing #11724

Merged
merged 5 commits into from
Feb 27, 2025

Conversation

bnussman-akamai
Copy link
Member

Description 📝

  • Makes a global change to our banner in order to improve the visuals of Cloud Manager
  • Makes fontSize of the CDS banner inherit the banner's imposed fontSize

Preview 📷

Before After
Screenshot 2025-02-25 at 12 17 37 PM Screenshot 2025-02-25 at 12 17 27 PM
Screenshot 2025-02-25 at 12 18 32 PM Screenshot 2025-02-25 at 12 18 23 PM

How to test 🧪

  • Verify this improves the UI of Cloud Manager and does not make things worse throughout the app
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

@bnussman-akamai bnussman-akamai added the UX/UI Changes for UI/UX to review label Feb 25, 2025
@bnussman-akamai bnussman-akamai self-assigned this Feb 25, 2025
@bnussman-akamai bnussman-akamai requested a review from a team as a code owner February 25, 2025 17:20
@bnussman-akamai bnussman-akamai requested review from mjac0bs and pmakode-akamai and removed request for a team February 25, 2025 17:20
@@ -143,7 +143,7 @@ export const Notice = (props: NoticeProps) => {
marginBottom:
spacingBottom !== undefined
? `${spacingBottom}px`
: theme.spacing(3),
: theme.spacing(1),
Copy link
Member Author

Choose a reason for hiding this comment

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

This does affect all notices throughout the application unless they specify a spacingBottom manually.

Ideally, our components don't have any spacing by default and the spacing is up to the component that contains the Notice, but I don't want to make such a big breaking change right now

Copy link

github-actions bot commented Feb 25, 2025

Coverage Report:
Base Coverage: 80.09%
Current Coverage: 80.09%

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.

Can we get a Jira ticket and a changeset for this one? Ticket may be slightly pedantic for a 3 line change, but Makes a global change to our banner is best if it's easily trackable.

Pending that feedback, I think this does improve the spacing in the UI for the better. Clicked around through various products to trigger notices and didn't see any glaring regressions. 👍🏼

@mjac0bs mjac0bs added Missing Changeset Add'tl Approval Needed Waiting on another approval! labels Feb 26, 2025
@bnussman-akamai bnussman-akamai changed the title chore: Improve Banner Spacing chore: [M3-9440] - Improve Banner Spacing Feb 26, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 530 passing tests on test run #5 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing530 Passing3 Skipped102m 37s

@pmakode-akamai pmakode-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Feb 27, 2025
@bnussman-akamai bnussman-akamai merged commit 63a250c into linode:develop Feb 27, 2025
24 checks passed
Copy link

cypress bot commented Feb 27, 2025

Cloud Manager E2E    Run #7309

Run Properties:  status check passed Passed #7309  •  git commit 63a250c065: chore: [M3-9440] - Improve Banner Spacing (#11724)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #7309
Run duration 28m 09s
Commit git commit 63a250c065: chore: [M3-9440] - Improve Banner Spacing (#11724)
Committer Banks Nussman
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
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 530
View all changes introduced in this branch ↗︎

aaleksee-akamai pushed a commit to aaleksee-akamai/manager that referenced this pull request Feb 27, 2025
* improve-banner-spacing

* update test

* add changeset

---------

Co-authored-by: Banks Nussman <[email protected]>
aaleksee-akamai pushed a commit to aaleksee-akamai/manager that referenced this pull request Feb 28, 2025
* improve-banner-spacing

* update test

* add changeset

---------

Co-authored-by: Banks Nussman <[email protected]>
aaleksee-akamai pushed a commit to aaleksee-akamai/manager that referenced this pull request Feb 28, 2025
* improve-banner-spacing

* update test

* add changeset

---------

Co-authored-by: Banks Nussman <[email protected]>
aaleksee-akamai pushed a commit to aaleksee-akamai/manager that referenced this pull request Feb 28, 2025
* improve-banner-spacing

* update test

* add changeset

---------

Co-authored-by: Banks Nussman <[email protected]>
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! UX/UI Changes for UI/UX to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants