feat(Footer)!: add FooterMenu component to replace PageMenu in Footer #1871
Conversation
…ng and tests. Deprecation of PageMenu
…ibility improvements
| } | ||
|
|
||
| .ams-footer-menu__link svg { | ||
| color: currentColor; |
| describe('Page menu', () => { | ||
| it('renders a page menu with children', () => { | ||
| const { container } = render( | ||
| <FooterMenu> |
There was a problem hiding this comment.
Using Footer.Menu etc. here may be slightly more realistic – we do so in other subcomponents.
| } | ||
|
|
||
| .ams-footer__menu { | ||
| align-items: center; |
There was a problem hiding this comment.
I don't think this is necessary
| font-family: var(--ams-footer-menu-link-font-family); | ||
| font-size: var(--ams-footer-menu-link-font-size); | ||
| font-weight: var(--ams-footer-menu-link-font-weight); | ||
| gap: var(--ams-footer-menu-link-gap); |
There was a problem hiding this comment.
This doesn't do anything anymore
| } | ||
|
|
||
| .ams-footer-menu__link:hover, | ||
| .ams-footer-menu__button:hover { |
There was a problem hiding this comment.
We don't have buttons in the footer menu
| @@ -1,4 +1,3 @@ | |||
| export { Footer } from './Footer' | |||
| export type { FooterProps } from './Footer' | |||
| export type { FooterBottomProps } from './FooterBottom' | |||
| export type { FooterTopProps } from './FooterTop' | |||
| export type { FooterSpotlightProps } from './FooterSpotlight' | |||
There was a problem hiding this comment.
We probably also want to export FooterMenuProps and FooterMenuLinkProps here
| <Heading className="ams-visually-hidden" level={2}> | ||
| Over deze website | ||
| </Heading>, | ||
| <Footer.Menu> |
There was a problem hiding this comment.
This should also get a key
| <Heading className="ams-visually-hidden" level={2}> | ||
| Over deze website | ||
| </Heading>, | ||
| <Footer.Menu> |
| @@ -1,7 +1,29 @@ | |||
| { | |||
| "ams": { | |||
| "footer": { | |||
| "top": { | |||
| "menu": { | |||
| "column-gap": { "value": "{ams.space.grid.md}" }, | |||
There was a problem hiding this comment.
Should be ams-space-lg I think, same as the Header
| "top": { | ||
| "menu": { | ||
| "column-gap": { "value": "{ams.space.grid.md}" }, | ||
| "row-gap": { "value": "{ams.space.grid.xs}" }, |
There was a problem hiding this comment.
Should be ams-space-xs I think
| text-underline-offset: var(--ams-footer-menu-link-text-underline-offset); | ||
| touch-action: manipulation; | ||
|
|
||
| @include text-rendering; |
There was a problem hiding this comment.
Can you add the hyphenation mixin here?
Describe the pull request
Thank you for contributing to the project!
Please use this template to help us handle your PR smoothly.
What
Why
PageMenu no longer needed and could be simplified by taking away the need for the Grid component.
How
PageMenu in Footer.Bottom replace with Footer.Menu, replaced Grid with padding.
Checklist
Before submitting your pull request, please ensure you have done the following. Check each checkmark if you have done so or if it wasn't necessary:
Additional notes