Skip to content

Conversation

@catheraaine
Copy link
Collaborator

@catheraaine catheraaine commented Apr 23, 2025

Summary of changes

  • Footer
  • Footer-Heading
  • Footer-Links
  • Copyright
  • Bypass blocks

Relevant Links

Test URLs:

Validation

Review the Footer component

  1. Head to any page
  2. Review the footer
  3. This guy has some spicy breakpoints, so very slowly review those on both dark and light mode
  4. The links should work for us now, including the "Adobe Design" logo, which is a link!
  5. Head to another page
  6. Still footer

Review the Skip Link component

  1. Included in this work we also added a Skip Link to the main navigation and a bypass block to the footer links.
  2. To get to this link, you have to give it your (keyboard) focus. So tab into your browser, or if that's a little weird, click on the page and shift-tab backward through the nav. The link should show up top and center only while it has keyboard focus. Hitting your enter key should navigate to the main content.
  3. I added an ID to every page to make this link work, so maybe check a few pages to see if that <main> element has an id and shine a blacklight on that thing to make sure it's a real ID and not one I just made in my garage and sold to the element for $15.
  4. Don't worry about the markup on this one too much, I've got a lot of work to do in the navigation already.
  5. The footer links also should have a bypass block. So if you haven't tabbed through those yet, see if you can skip the links and go back to the top of the main content.

@aem-code-sync
Copy link

aem-code-sync bot commented Apr 23, 2025

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@catheraaine catheraaine added the WIP Work in progress, likely a draft. label Apr 23, 2025
@aem-code-sync
Copy link

aem-code-sync bot commented Apr 23, 2025

Page Scores Audits Google
📱 / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@catheraaine catheraaine requested a review from arnest00 April 25, 2025 04:11
@catheraaine catheraaine added Ready for Dev Review and removed WIP Work in progress, likely a draft. labels Apr 25, 2025
@catheraaine catheraaine marked this pull request as ready for review April 25, 2025 04:11
skipToContentLink.innerText = "Skip to main content";

return skipToContentLink;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a brilliant addition to our toolbox.

feat: footer links

feat: footer copyright

feat: main content has an id

feat: main nav has a skip link

feat: footer has bypass block

feat: blocks-helpers houses helper scripts
Copy link
Collaborator

@arnest00 arnest00 left a comment

Choose a reason for hiding this comment

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

Fantastic work on this. Just one question but otherwise I think we're about ready for a rocket-shaped emoji.

}
}

/* ** Typography ** */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for labeling these sections.

display: block;
max-width: 9.5rem;
color: var(--color-text-dark);
margin-bottom: 1rem;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, covering the case if there were ever no footer-links.

&:has(#color-scheme option[value="dark"]:checked) {
--color-background-footer: var(--spectrum-gray-50);
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We've applied the anti-aliasing properties on the body element in base.css, is stating it here redundant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it sure is

Copy link
Collaborator

@arnest00 arnest00 left a comment

Choose a reason for hiding this comment

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

Vielen dank, Catherine 🚀

@catheraaine catheraaine merged commit 6fc59fe into main Apr 29, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants