-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ui): improve Footer styling #1328
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 923532b The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
| /* LT Box Shadow */ | ||
| --box-shadow-default: 0 1px 2px 0 rgba(34, 54, 73, 0.3); | ||
| --box-shadow-hover: 0 0 0 1px var(--color-color-shadow, rgba(79, 79, 79, 0.3)), 0 2px 8px 0 rgba(0, 0, 0, 0.3); | ||
| /* LT PageFooter */ |
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.
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.
The same issue doesn't appear in Storybook, right? It doesn't seem to be an issue with the component? This is Example App's custom styling, I'll work on it. Good reminder to update Example App.
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.
Yes, actually i did not thought much further, as its default then most likly to light theme also in other places and apps - what is maybe not wanted, or at least kinda breaking. But as I said, i did not think much further about it.
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 issue occurred as in AC we expected the footer background to be transparent. However, since it has no parent, it always defaulted the white. I defined the exact colour for each theme and updated the current ticket we have to improve AppShell layout.
Thank You for the catch.
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.
Love the bug joke btw, hahaha!!!
packages/ui-components/src/components/PageFooter/PageFooter.stories.tsx
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/PageFooter/page-footer.css
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/PageFooter/page-footer.css
Outdated
Show resolved
Hide resolved
Away, other reviewers please double check.
| text-decoration: none; | ||
| } | ||
|
|
||
| .juno-content { |
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.
The name of this class is very generic. It doesn't sound like it's a class for a specifc child of juno-pagefooter. My suggestion: rename to .juno-pagefooter-content.



Summary
Enhance the
PageFootercomponent to accommodate a wider range of content and facilitate flexible theming. This update improves both accessibility and design consistency across different usage scenarios within our design system.Changes Made
role="group") and attributes (aria-labelledby) to improve accessibility and semantic clarity in sectioning.PageFooterto Example App.Follow up
AppShellticket: #1345Testing Instructions
Compare to design and ACs in Storybook.
Checklist
PR Manifesto
Review the PR Manifesto for best practises.