ic-ui-kit icon indicating copy to clipboard operation
ic-ui-kit copied to clipboard

1985 top nav groups and items not in line

Open GCHQ-Developer-773 opened this issue 1 year ago • 5 comments

Summary of the changes

Aligning the nav group in the nav bar with the other nav items, by changing the padding of the nav group.

Related issue

1985

Checklist

General

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

Testing

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

Accessibility

  • [x] Accessibility Insights FastPass performed.
  • [x] A11y unit test added and yields no issues.
  • [x] A11y plug-in on Storybook yields no issues.
  • [x] Manual screen reader testing performed using NVDA and VoiceOver.
  • [x] Manual keyboard testing for keyboard controls and logical focus order.
  • [x] Correct roles used and ARIA attributes used correctly where required.
  • [x] 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

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

System modes

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

Testing content extremes

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

GCHQ-Developer-773 avatar Oct 02 '24 10:10 GCHQ-Developer-773

View your branch deployment here: https://mi6.github.io/ic-ui-kit/branches/1985-top-nav-groups-and-items-not-in-line/web-components View your React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/1985-top-nav-groups-and-items-not-in-line/react

github-actions[bot] avatar Oct 02 '24 10:10 github-actions[bot]

Have the new baseline images been generated locally or on the CI?

gd2910 avatar Oct 02 '24 14:10 gd2910

Have the new baseline images been generated locally or on the CI?

They were generated through the ci on the previous version of this branch, and copied over to this one, should I regenerate them on the CI?

GCHQ-Developer-773 avatar Oct 02 '24 14:10 GCHQ-Developer-773

I'm still concerned about the 5% visual regression threshold increase on each image. The threshold is to take care of font differences between machines, but there is no change in the PR. When regenerating the images did you note where the regression differences were coming through?

gd2910 avatar Oct 08 '24 11:10 gd2910

I think that the extra threshold is to account for the padding making the canvas height a few pixels larger, this shouldn't be the case if the baseline images used were generated on the CI.

gd2910 avatar Oct 08 '24 14:10 gd2910

yeah, I'm not sure the thresholds need increasing. Running locally on a windows machine, they pass & the actual difference is quite a bit lower than the actual threshold in some cases. It would be good to set them as low as possible to increase chances of catching genuine regressions locally before committing

image image

ad9242 avatar Oct 16 '24 07:10 ad9242

Cypress visual tests failed. View the image diff here: https://github.com/mi6/ic-ui-kit/tree/gh-pages/branches/1985-top-nav-groups-and-items-not-in-line/cypress-image-diff-screenshots/diff View the html report here: https://mi6.github.io/ic-ui-kit/branches/1985-top-nav-groups-and-items-not-in-line/cypress-image-diff-html-report/cypress-image-diff-html-report.html

github-actions[bot] avatar Oct 16 '24 07:10 github-actions[bot]