Skip to content

#666 ic-select refactor #4219

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.

Sign up for GitHub

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

Jump to bottom
Open
lz405 wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

#666 ic-select refactor #4219

lz405 wants to merge 5 commits into from

Conversation

@lz405
Copy link
Contributor

@lz405 lz405 commented Jan 28, 2026

Summary of the changes

Refactor ic-select code #666
Update ic-select and ic-menu to follow the combobox pattern #3757

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

View your branch deployment here: https://mi6.github.io/ic-ui-kit/branches/666-refactor-branch/web-components
View your React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/666-refactor-branch/react

GCHQ-Developer-299
GCHQ-Developer-299 reviewed Jan 28, 2026
View reviewed changes
packages/web-components/src/components/ic-select/ic-select.tsx
} from "@stencil/core";

import {
getInputDescribedByText,
Copy link
Contributor

@GCHQ-Developer-299 GCHQ-Developer-299 Jan 28, 2026

Choose a reason for hiding this comment

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

your PR is now failing test coverage for utils/helpers - I think because you've removed the usage of some helper functions in ic-select, so they're not being tested in select's jest tests. Are there helper functions that are not getting used now, and can be deleted? If not you might need to write tests for them

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

Cypress visual tests failed.
View the image diff here: https://github.com/mi6/ic-ui-kit/tree/gh-pages/branches/666-refactor-branch/cypress-image-diff-screenshots/diff
View the html report here: https://mi6.github.io/ic-ui-kit/branches/666-refactor-branch/cypress-image-diff-html-report/cypress-image-diff-html-report.html

@lz405 lz405 force-pushed the 666-refactor-branch branch 2 times, most recently from 685fd85 to acecb8f Compare January 29, 2026 15:29
lz405 added 5 commits January 29, 2026 15:33
@lz405
refactor(web-components): refactor ic-select to simplify code and fol…
439d238 
…low combobox pattern

refactor ic-select to simplify code and follow the combobox pattern

re #666
@lz405
test(react): update cypress tests for ic-select
f41cb34 
update cypress tests for ic-select

test #666
@lz405
test(react): update visual regression tests
028a8ac 
update visual regression tests

test #666
@lz405
chore(root): add word
7435248
@lz405
docs(docs): update docs for ic-select
562a741 
update docs for ic-select

docs #666
@lz405 lz405 force-pushed the 666-refactor-branch branch from acecb8f to 562a741 Compare January 29, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GCHQ-Developer-299 GCHQ-Developer-299 GCHQ-Developer-299 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lz405 @GCHQ-Developer-299