-
Notifications
You must be signed in to change notification settings - Fork 860
Reduce EuiEmptyPrompt spacing in Borealis theme
#9044
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
Conversation
- Decrease title-to-body spacing from 'm' to 's' (16px → 8px) - Decrease body-to-actions spacing from 'l' to 'm' (24px → 16px) - Decrease default padding from 'l' to 'm' (24px → 16px) - Create _spacing.ts file following established theme patterns - Extend EuiThemeShape to include custom spacing property - Update both light and dark Borealis theme JSON files - Modify EuiEmptyPrompt to use theme-based spacing values - Update snapshot tests to reflect new spacing values - Amsterdam theme remains unchanged Addresses #9039
EuiEmptyPrompt spacing in Borealis theme
|
@ryankeairns awesome! 🚀 One thing we're missing is the VRT images. Could you update them by running |
|
@ryankeairns btw we'd need a changelog for |
|
Thanks @weronikaolejniczak , will do! |
- Update .loki reference images to reflect reduced spacing in Borealis theme - Add changelog entry for emptyPrompt spacing theme defaults
|
@weronikaolejniczak ready for another look! |
💚 Build SucceededHistory
|
💚 Build Succeeded
History
|
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.
@ryankeairns I'm thinking, we have the "Components" tokens, maybe it'd be better to add an emptyPrompt property there with the spacing values? 🤔 I believe it'd be more consistent and expected. We would have to update the documentation while we're at it.
Let me know what you think! And sorry for the back-and-forth, I just believe it's worth mentioning 💚
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 you explain that further?
No worries on the back and forth. I'm hoping to find some patterns as I intend to do more theme overrides.
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.
So it would be something like adding these new tokens:
euiTheme.components.emptyPrompt.titleBodySpacingeuiTheme.components.emptyPrompt.bodyActionsSpacingeuiTheme.components.emptyPrompt.paddingSize
... and then wiring those in?
Then adding a section to the docs per component? (e.g. as we have Buttons today; or maybe a Components section?)
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.
@weronikaolejniczak Opened a new PR, based on the same branch, that reworks to use theme component tokens
#9057
|
@ryankeairns did you have a chance to run |
Yes, it added one updated png. Should there be more? I can run it again. |
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.
@weronikaolejniczak here's the sole visual regression that was added.
|
Reverted to Draft. |
|
closing in favor of #9057 |
Closes #9039
Summary
For the Borealis theme:
_spacing.tsfile following established theme config patternsEuiThemeShapeto include custom spacing propertyEuiEmptyPromptto use new theme-based spacing valuesAmsterdam theme remains unchanged
Screenshots
Borealis before (left) and after (right)
Amsterdam before and after (unchanged)
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
@defaultif default values are missing) and playground toggles