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

3010 date input extend events #3345

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

Conversation

cd3859
Copy link
Contributor

@cd3859 cd3859 commented Mar 20, 2025

Summary of the changes

Added a dateObject field to IcChange return detail, IcChange is now emitted after every section of input date is completed

Related issue

#3010

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

@cd3859 cd3859 force-pushed the 3010-date-input-extend-events branch from 99cf69f to ef9d38d Compare March 20, 2025 14:32
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 good overall but have just found a couple of issues (while testing in the “icChange event” story):



  • If you only type in part of the date, e.g. the day, and then delete it by either pressing backspace or clicking the clear button, the icChange event does not get emitted to say that that section is now empty again

  • The sections sometimes get set to null and other times an empty string. E.g. if you only type in the day, the month and year sections show as an empty string in the icChange event. But if you type in only the year and then click the clear button, the icChange event gets emitted saying that the day and month are null. I think it should be consistent i.e. either they only ever get emitted as null when cleared / deleted, or only as an empty string (maybe null would be best to match the value behaviour?)

@cd3859 cd3859 force-pushed the 3010-date-input-extend-events branch 2 times, most recently from 89b4a14 to 0f200c8 Compare March 25, 2025 13:58
@cd3859 cd3859 force-pushed the 3010-date-input-extend-events branch from 0f200c8 to 340380f Compare March 25, 2025 14:05
@ad9242
Copy link
Contributor

ad9242 commented Mar 25, 2025

just wondering if changing this to now emit for each field is going to cause developers issues? previously they would get 1 icChange event once the date was complete\the field blurred, but now they will be getting multiple as the user types & will need to adjust their code to account for this

eg
user types 01 in day field:
icChange triggered with value null and day value 01

user than types 02 in month field:
icChange triggered with value null, day value 01 and month value 02

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.

3 participants