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

Add ability to hide the IcDateInput control in the IcDatePicker #3333

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

dn55533
Copy link
Contributor

@dn55533 dn55533 commented Mar 18, 2025

Summary of the changes

Add ability to hide the IcDateInput control in the IcDatePicker

This allows the IcDatePicker component to be used as a more generic date picking calendar, without the added complexity of entering dates in the IcDateInput.

This makes the component more generic and allows the calendar and date picking part of the component to be used as a building block in other date-related components.

Calendar

Related issue

N/A

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.

dn55533 added 2 commits March 18, 2025 19:07
…atepicker

add a new showDateInput prop allowing the date input to be hidden and the calendar to always be open
…in the icdatepicker

add a new showDateInput prop allowing the date input to be hidden and the calendar to always be open
@ad9242
Copy link
Contributor

ad9242 commented Mar 19, 2025

Hi @dn55533, thanks for the contribution. we will run it past the design team as the figma library may also require an update to match the behaviour.

Also would with the release of v3, would you mind carrying out this work in the v3.0.0/develop code base, as v2 will only be for major bug fixes soon. You are welcome to contribute it to v2 too, but just a heads up that the code bases are already slightly different, so may require some duplication of effort

@dn55533 dn55533 force-pushed the feat/show-ic-date-picker-calendar-without-ic-date-input branch from c72d898 to 53c82c0 Compare March 19, 2025 07:45
…datepicker

add a new showDateInput prop allowing the date input to be hidden and the calendar to always be open
@dn55533 dn55533 force-pushed the feat/show-ic-date-picker-calendar-without-ic-date-input branch from 53c82c0 to d5c57d9 Compare March 19, 2025 07:47
@dn55533
Copy link
Contributor Author

dn55533 commented Mar 19, 2025

Hi @dn55533, thanks for the contribution. we will run it past the design team as the figma library may also require an update to match the behaviour.

Also would with the release of v3, would you mind carrying out this work in the v3.0.0/develop code base, as v2 will only be for major bug fixes soon. You are welcome to contribute it to v2 too, but just a heads up that the code bases are already slightly different, so may require some duplication of effort

Sure, happy to make the change in V3 as well. If you could review this V2 version, once you're happy with it I'll port it to V3.

@ad9242
Copy link
Contributor

ad9242 commented Mar 19, 2025

We're still wanting to run this past the design team, and it may take some time to get eyes on it, but we do have a few initial concerns. The date picker isn't currently intended to be used without the input field. The definition of it is: "A date picker is a type of drop-down control that allows people to easily view and select dates."

Our initial thoughts are that trying to just use the calendar view part might be better suited as a new component (which the date-picker could then potentially be refactored to use).

There are a few reasons for this:

  • A number of props no longer make sense as they are passed straight through to the input control: helper text, label, validation messages, disabled & required are some quick ones that come to mind
  • the accessibility of the datepicker & interaction with a screen reader currently requires the date-input as this is how the selected date is communicated to a screen reader

@dn55533 dn55533 marked this pull request as draft March 20, 2025 08:28
@dn55533
Copy link
Contributor Author

dn55533 commented Mar 20, 2025

Converted PR to draft to carry out the refactoring of the calendar into a separate component and refactoring of the date picker to use it.

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.

2 participants