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

2151 skip to content component #3101

Open
wants to merge 3 commits into
base: v3.0.0/develop
Choose a base branch
from

Conversation

GCHQ-Developer-741
Copy link
Contributor

@GCHQ-Developer-741 GCHQ-Developer-741 commented Jan 28, 2025

NOTE: Do not merge until guidance is ready.

Summary of the changes

  • Added the new ic-skip-link component to the web-components package (final confirmation from design still needed).
  • Added stories to both web-components and react packages, including prop playgrounds.
  • Added new style token to global styles relating to the new component.
  • Added unit/cypress tests.

Related issue

#2151

Checklist

General

  • Changes to docs package checked and committed.
  • All acceptance criteria reviewed and met.

Testing

  • Relevant unit tests and visual regression tests added.
  • Visual testing against Figma component specification completed.
  • Playground stories in React Storybook up to date, with any prop changes and additions addressed.
  • Compare performance of modified components against develop using Performance addon in React Storybook.

Accessibility

  • Accessibility Insights FastPass performed.
  • A11y unit test added and yields no issues.
  • A11y plug-in on Storybook yields no issues.
  • Manual screen reader testing performed using NVDA and VoiceOver.
  • Manual keyboard testing for keyboard controls and logical focus order.
  • Correct roles used and ARIA attributes used correctly where required.
  • Logical heading structure is maintained, and the HTML elements used for headings can be changed to fit within the wider page structure.

Resize/zoom behaviour

  • Page can be zoomed to 400% with no loss of content.
  • Screen magnifier used with no issues.
  • Text resized to 200% with no loss of content.
  • Text spacing increased as per the WCAG 1.4.12 success criterion with no loss of content.

System modes

  • Browser setting 'prefers reduced motion' tested. No animations or motion visible whilst this setting is on.
  • Windows High Contrast mode tested with no loss of content.
  • System light and dark mode tested with no loss of content.
  • Browser support tested (Chrome, Safari, Firefox and Edge).

Testing content extremes

  • Min/max content examples tested with no loss of content or overflow.
  • All prop combinations work without issue.
  • Tested for FOUC (Flash of Unstyled Content) in both SSR (Server-Side Rendering) and SSG (Static Site Generation) settings.
  • Controlled and uncontrolled input components tested.
  • Props/slots can be updated after initial render.

Copy link
Contributor

@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch from c9d0690 to 39cd82d Compare January 28, 2025 12:29
@GCHQ-Developer-741 GCHQ-Developer-741 marked this pull request as ready for review January 28, 2025 12:53
@GCHQ-Developer-741 GCHQ-Developer-741 marked this pull request as draft January 28, 2025 12:53
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch from 39cd82d to efe3184 Compare January 28, 2025 15:55
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 4 times, most recently from ffd4cbe to e7c07d4 Compare January 30, 2025 13:33
@GCHQ-Developer-741 GCHQ-Developer-741 linked an issue Jan 30, 2025 that may be closed by this pull request
@GCHQ-Developer-741 GCHQ-Developer-741 marked this pull request as ready for review February 3, 2025 11:55
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 6 times, most recently from 0dd754d to a794d67 Compare February 5, 2025 09:28
@mi6 mi6 deleted a comment from github-actions bot Feb 5, 2025
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch from a794d67 to d67a949 Compare February 7, 2025 12:36
Copy link
Contributor

@GCHQ-Developer-847 GCHQ-Developer-847 left a comment

Choose a reason for hiding this comment

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

Looks so good, noticed a few things but they are quite small :)

@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 2 times, most recently from fbef52f to 0486758 Compare February 18, 2025 10:46
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 3 times, most recently from 713b877 to f71be7a Compare February 18, 2025 12:01
@GCHQ-Developer-847
Copy link
Contributor

I'm still getting an issue with the full-width variant. It appears correctly if you keep the window at the same size, but once you make it bigger or smaller, the skip link no longer matches the window width i.e. it either gets cut off or is not wide enough

Everything else looks good to me now though :)

@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 5 times, most recently from 8f3440b to 8a04dce Compare February 19, 2025 15:20
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch 2 times, most recently from c49d01f to 57bb3a7 Compare February 20, 2025 10:22
@GCHQ-Developer-299 GCHQ-Developer-299 force-pushed the v3.0.0/develop branch 2 times, most recently from 8d14ba9 to 566d663 Compare February 20, 2025 12:23
@MI6-255 MI6-255 dismissed GCHQ-Developer-847’s stale review February 24, 2025 11:34

The merge-base changed after approval.

@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch from 57bb3a7 to f98a7dc Compare February 24, 2025 14:39
added an ic-skip-link component, including relevant props, styling and stories. Included spec tests,
and added some new color tokens to global css files
added new component definition to react package for ic-skip-link. Also added stories and cypress
tests
added ic-skip-link to docs package
@GCHQ-Developer-741 GCHQ-Developer-741 force-pushed the 2151-skip-to-content-component branch from f98a7dc to 49d5781 Compare February 25, 2025 14:54
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.

"Skip to content" link component
3 participants