-
Notifications
You must be signed in to change notification settings - Fork 370
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
chore: [M3-9440] - Improve Banner Spacing #11724
Conversation
@@ -143,7 +143,7 @@ export const Notice = (props: NoticeProps) => { | |||
marginBottom: | |||
spacingBottom !== undefined | |||
? `${spacingBottom}px` | |||
: theme.spacing(3), | |||
: theme.spacing(1), |
There was a problem hiding this comment.
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
Coverage Report: ✅ |
There was a problem hiding this 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. 👍🏼
Cloud Manager UI test results🎉 530 passing tests on test run #5 ↗︎
|
Cloud Manager E2E
|
Project |
Cloud Manager E2E
|
Branch Review |
develop
|
Run status |
|
Run duration | 28m 09s |
Commit |
|
Committer | Banks Nussman |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
3
|
|
3
|
|
0
|
|
530
|
View all changes introduced in this branch ↗︎ |
* improve-banner-spacing * update test * add changeset --------- Co-authored-by: Banks Nussman <[email protected]>
* improve-banner-spacing * update test * add changeset --------- Co-authored-by: Banks Nussman <[email protected]>
* improve-banner-spacing * update test * add changeset --------- Co-authored-by: Banks Nussman <[email protected]>
* improve-banner-spacing * update test * add changeset --------- Co-authored-by: Banks Nussman <[email protected]>
Description 📝
Preview 📷
How to test 🧪
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
As an Author, before moving this PR from Draft to Open, I confirmed ✅