Skip to content
This repository has been archived by the owner on May 18, 2024. It is now read-only.

(fix): ui-toolkit - Calendar functionality and styling #326

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

Conversation

rohangupta-groww
Copy link

What does this PR do?

  1. Added date active, disabled and hover states.2
  2. Bounded calendar to min and max values
  3. Removed un-necessary elements and redundant code.
  4. Fixed disabled state in case of hover
  5. Added onMonthClick | onDateClickvalidation
  6. Extended props from MonthCalendar to DateCalendar
  7. Added highlightCurrentDate prop to show the current date
  8. Navigation bar has proper disabled state greyed-out in both calendar
  9. Changed type prop from string to DATE | MONTH with fallback to DATE
  10. Added missing key prop in several places to fix un-necessary re-renders.

These fixes were made specifically to address issues with calendar component across web_apps like

  1. Active states are not visible or shown, In second SS even though the above range selector shows the current date but that isn't present everywhere.
image image
  1. We are able to navigate across the max | min date value and shows pointer even on disabled state
image

After:

image image image image

Notes:

  1. There is no validation when max < min, Could have thrown an error if __DEV__ ?
  2. Storybook has an issue to validate types properly in controls, refer 4th image in After
  3. Props for both MonthCalendar and DateCalendar are now same, however there is no breaking change as props have been extended to additional functionality.
  4. highlightCurrentDate prop is true by default and will highlight the current value, Could be false?
  5. Have moved utils outside as compareDate function was needed in both
  6. Earlier active state was conditionally improperly https://github.com/Groww/webster/blob/37221681d7bb7e9552a5e5d52d27137868caa29a/packages/ui-toolkit/src/components/molecules/Calendar/DateCalendar/index.tsx#L146 this will never work until a re-render is triggered, currently onDateClick doesn't update any state so no re-render will trigger and active states won't be marked
    https://github.com/Groww/webster/blob/37221681d7bb7e9552a5e5d52d27137868caa29a/packages/ui-toolkit/src/components/molecules/Calendar/DateCalendar/index.tsx#L217-L233
    To solve this selectedDate: Date | null; was added in React State which is used in calendar dates and will trigger a re-render to mark the new selection date.
  7. An assumption was made for highlightCurrentDate an already existing color --green300 has been set and for selected date --green500

What packages have been affected by this PR?

  • ui-toolkit

Types of changes

What types of changes does your code introduce to this project?

Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Package version increase in any of the packages?

  • ui-toolkit -> 0.4.6

Checklist before merging

Put an x in the boxes that apply

  • These changes have been thoroughly tested.

  • Changes need to be immediately published on npm.

@rohangupta-groww rohangupta-groww added bug Something isn't working refactor Code cleanup / Updating legacy code labels Sep 15, 2023
@rohangupta-groww rohangupta-groww self-assigned this Sep 15, 2023
Copy link
Contributor

@Amit-Groww Amit-Groww left a comment

Choose a reason for hiding this comment

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

discussed few minor changes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working refactor Code cleanup / Updating legacy code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants