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

Feature: Improve footer content #18

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Feature: Improve footer content #18

wants to merge 8 commits into from

Conversation

dbrosio3
Copy link
Owner

@dbrosio3 dbrosio3 commented May 26, 2022

Links:

What & Why:

Grabación de pantalla 2022-05-26 a la(s) 17 34 38

Risks, Mitigation & Rollback:

  • Safe to Revert check this box if this PR can be reverted without incident

@dbrosio3 dbrosio3 changed the title add footer content Add footer content May 26, 2022
@dbrosio3 dbrosio3 changed the title Add footer content Improve footer content May 26, 2022
@dbrosio3 dbrosio3 force-pushed the feature/footer branch 2 times, most recently from 48422fc to c8b767e Compare May 26, 2022 21:08
@dbrosio3 dbrosio3 changed the title Improve footer content Feature: Improve footer content Jun 16, 2022
Base automatically changed from feature/app-layout to develop June 21, 2022 18:41
@dbrosio3 dbrosio3 requested a review from aguscha333 as a code owner June 21, 2022 18:41
<VStack
w="100%"
justify="space-between"
flexDirection={['column-reverse', 'column-reverse', 'column']}

Choose a reason for hiding this comment

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

'column-reverse' twice here, is that correct?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's correct. The array syntax applies the first value until the sm breakpoint, the second value until the md breakpoint, and so on...

src/components/Footer/NewsletterSection.tsx Outdated Show resolved Hide resolved

import { MaxWidthWrapper } from '@components/Misc';

import { FooterContainer } from './FooterContainer';
import { LinksSection } from './LinksSection';
import { LogosSection } from './LogosSection';
import { NewsletterSection } from './NewsletterSection';

export const Footer = () => (

Choose a reason for hiding this comment

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

I think chakra-ui is already contemplating the accessibility part but this is an interesting example of an accessible Footer, you can take a look: https://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-navigation-footer
There you can see that is using the correct semantic for the Footer and the lists inside it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

That's very helpful. It should be fixed now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants