-
Notifications
You must be signed in to change notification settings - Fork 56
feat: refactor date picker components and remove deprecated code #3003
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
base: develop
Are you sure you want to change the base?
Conversation
- Introduced a new DateTimePicker component to replace the existing SingleDatePicker and DayPickerRangeController. - Updated DeploymentMetrics and CustomLogsModal to utilize the new DateTimePicker for date selection. - Removed unused styles and components related to the old date picker implementation. - Added utility functions for generating date range options for the new date picker. - Cleaned up imports and adjusted type definitions to accommodate the new date handling logic.
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
…, GraphModal, and DeploymentMetrics components
| startDate: startStr, | ||
| endDate: 'now', | ||
| }) | ||
| const str = getCalendarValue(startStr, 'now') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to check if now-5m and now - 5m are same?
…ency and readability
…nto feat/day-picker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors date handling across the application, migrating from react-dates and custom date picker components to a standardized DateTimePicker component from the common library. The changes improve type safety and maintainability by using dayjs and shared types instead of moment and custom implementations.
Key Changes
- Replaced
react-datesdependency withreact-day-picker(via common library update to v1.21.0-beta-2) - Migrated API token expiration date handling to use standardized
ExpirationDateSelectOptionTypeand sharedDateTimePickercomponent - Refactored app metrics and deployment metrics to use the new
DateTimePickerwith proper range selection and preset options - Removed deprecated custom calendar components and associated CSS
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated common library to v1.21.0-beta-2 and removed react-dates dependency |
| yarn.lock | Updated dependency lockfile with new common library version and removed react-dates related packages |
| src/config/constants.ts | Removed unused MomentDateFormat constant |
| src/components/common/index.ts | Removed export of deprecated calendar components |
| src/components/common/DatePickers/calendar.css | Deleted deprecated custom calendar styles |
| src/components/common/DatePickers/Calender.tsx | Deleted deprecated custom calendar component |
| src/components/app/details/metrics/utils.ts | Added utility functions for date range shortcut options |
| src/components/app/details/metrics/deploymentMetrics.types.ts | Removed focusedInput state property |
| src/components/app/details/metrics/DeploymentMetrics.tsx | Migrated to DateTimePicker with range selection and shortcuts |
| src/components/app/details/appDetails/utils.tsx | Refactored calendar utilities to use preset options pattern |
| src/components/app/details/appDetails/types.ts | Added GrafanaPresetOptionHandlerType type definition |
| src/components/app/details/appDetails/appDetails.type.ts | Removed deprecated CalendarFocusInput enum and type |
| src/components/app/details/appDetails/GraphsModal.tsx | Refactored to use DateTimePicker with range selection |
| src/components/app/details/appDetails/AppMetrics.tsx | Refactored to use DateTimePicker with range selection |
| src/components/v2/appDetails/k8Resource/nodeDetail/NodeDetailTabs/CustomLogsModal/CustomLogsModal.tsx | Migrated to DateTimePicker component |
| src/Pages/GlobalConfigurations/Authorization/APITokens/types.ts | Added ExpirationDateSelectOptionType and ExpirationDateProps |
| src/Pages/GlobalConfigurations/Authorization/APITokens/apiToken.utils.ts | Updated getOptions to use typed SelectPickerOptionType |
| src/Pages/GlobalConfigurations/Authorization/APITokens/apiToken.type.ts | Added type annotations for expiration date props |
| src/Pages/GlobalConfigurations/Authorization/APITokens/RegenerateModal.tsx | Updated to use typed expiration date handling |
| src/Pages/GlobalConfigurations/Authorization/APITokens/ExpirationDate.tsx | Migrated to DateTimePicker and DATE_TIME_FORMATS |
| src/Pages/GlobalConfigurations/Authorization/APITokens/EditAPIToken.tsx | Updated date formatting to use DATE_TIME_FORMATS |
| src/Pages/GlobalConfigurations/Authorization/APITokens/CreateAPIToken.tsx | Updated to use typed expiration date handling |
| src/Pages/GlobalConfigurations/Authorization/APITokens/ApiTokens.component.tsx | Added type annotation for selectedExpirationDate |
| src/Pages/GlobalConfigurations/Authorization/APITokens/APITokenList.tsx | Updated date formatting to use DATE_TIME_FORMATS |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| meanRecoveryTime: 0, | ||
| benchmarkModalData: undefined, | ||
| startDate: moment().set({ hour: 0, minute: 0, seconds: 0 }).subtract(6, 'months'), | ||
| endDate: moment().set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seconds property should be second (singular) when using moment's set() method. Moment.js uses singular forms for time unit properties.
Change to:
endDate: moment().set({ hour: 23, minute: 59, second: 59, millisecond: 999 }),Also, milliseconds should be millisecond (singular).
| endDate: moment().set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), | |
| endDate: moment().set({ hour: 23, minute: 59, second: 59, millisecond: 999 }), |
| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, seconds: 0 }), | ||
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seconds and milliseconds properties should be singular (second and millisecond) when using moment's set() method. Moment.js uses singular forms for time unit properties.
Change to:
endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, second: 59, millisecond: 999 }),| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, seconds: 0 }), | |
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), | |
| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, second: 0 }), | |
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, second: 59, millisecond: 999 }), |
| {moment(getDateInMilliseconds(selectedExpirationDate.value)).format( | ||
| DATE_TIME_FORMATS.DD_MMM_YYYY_HH_MM, | ||
| )} |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function getDateInMilliseconds expects a number (days), but selectedExpirationDate.value can be either a number or a Date when "Custom" is selected. This will cause incorrect calculations or runtime errors when a custom date is selected.
The code should check the type before calling getDateInMilliseconds:
{moment(typeof selectedExpirationDate.value === 'number'
? getDateInMilliseconds(selectedExpirationDate.value)
: selectedExpirationDate.value
).format(DATE_TIME_FORMATS.DD_MMM_YYYY_HH_MM)}| {moment(getDateInMilliseconds(selectedExpirationDate.value)).format( | |
| DATE_TIME_FORMATS.DD_MMM_YYYY_HH_MM, | |
| )} | |
| {typeof selectedExpirationDate.value === 'number' | |
| ? moment(getDateInMilliseconds(selectedExpirationDate.value)).format(DATE_TIME_FORMATS.DD_MMM_YYYY_HH_MM) | |
| : ''} |
| const parsedMilliseconds = selectedOption.value === 0 ? 0 : getDateInMilliseconds(selectedOption.value) | ||
|
|
||
| setRegeneratedExpireAtInMs( | ||
| typeof selectedOption.value === 'number' ? parsedMilliseconds : selectedOption.value.valueOf(), | ||
| ) |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function getDateInMilliseconds is being called with selectedOption.value which can be either a number (days) or a Date object (when "Custom" is selected), but the function only accepts numbers. This will cause incorrect behavior.
The logic should be:
const parsedMilliseconds = selectedOption.value === 0
? 0
: typeof selectedOption.value === 'number'
? getDateInMilliseconds(selectedOption.value)
: selectedOption.value.valueOf()| const parsedMilliseconds = selectedOption.value === 0 ? 0 : getDateInMilliseconds(selectedOption.value) | |
| setRegeneratedExpireAtInMs( | |
| typeof selectedOption.value === 'number' ? parsedMilliseconds : selectedOption.value.valueOf(), | |
| ) | |
| const parsedMilliseconds = selectedOption.value === 0 | |
| ? 0 | |
| : typeof selectedOption.value === 'number' | |
| ? getDateInMilliseconds(selectedOption.value) | |
| : selectedOption.value.valueOf() | |
| setRegeneratedExpireAtInMs(parsedMilliseconds) |
| const onChangeSelectFormData = (selectedOption: { label: string; value: number }) => { | ||
| const onChangeSelectFormData = (selectedOption: ExpirationDateSelectOptionType) => { | ||
| setSelectedExpirationDate(selectedOption) | ||
| const parsedMilliseconds = selectedOption.value === 0 ? 0 : getDateInMilliseconds(selectedOption.value) |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function getDateInMilliseconds is being called with selectedOption.value which can be either a number (days) or a Date object (when "Custom" is selected), but the function only accepts numbers. This will cause incorrect behavior.
The logic should be:
const parsedMilliseconds = selectedOption.value === 0
? 0
: typeof selectedOption.value === 'number'
? getDateInMilliseconds(selectedOption.value)
: selectedOption.value.valueOf()| id="expiration-date-picker" | ||
| date={customDate} | ||
| onChange={handleDatesChange} | ||
| isTodayBlocked |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DateTimePicker component is missing the hideTimeSelect prop. Since this is for selecting an expiration date (not time), the time selection should be hidden for consistency with other date pickers in the codebase and to avoid confusion.
Add:
<DateTimePicker
id="expiration-date-picker"
date={customDate}
onChange={handleDatesChange}
isTodayBlocked
hideTimeSelect
/>| isTodayBlocked | |
| isTodayBlocked | |
| hideTimeSelect |
| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, seconds: 0 }), | ||
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seconds property should be second (singular) when using moment's set() method. Moment.js uses singular forms for time unit properties.
Change to:
startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, second: 0 }),| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, seconds: 0 }), | |
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), | |
| startDate: moment(newStartDate)?.set({ hour: 0, minute: 0, second: 0 }), | |
| endDate: moment(newEndDate)?.set({ hour: 23, minute: 59, second: 59, milliseconds: 999 }), |
| startDate: moment().set({ hour: 0, minute: 0, seconds: 0 }).subtract(6, 'months'), | ||
| endDate: moment().set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seconds property should be second (singular) when using moment's set() method. Moment.js uses singular forms for time unit properties.
Change to:
startDate: moment().set({ hour: 0, minute: 0, second: 0 }).subtract(6, 'months'),| startDate: moment().set({ hour: 0, minute: 0, seconds: 0 }).subtract(6, 'months'), | |
| endDate: moment().set({ hour: 23, minute: 59, seconds: 59, milliseconds: 999 }), | |
| startDate: moment().set({ hour: 0, minute: 0, second: 0 }).subtract(6, 'months'), | |
| endDate: moment().set({ hour: 23, minute: 59, second: 59, milliseconds: 999 }), |
…X in SSOLogin and EphemeralContainerDrawer fix: update workflowId to ciPipelineId in BranchRegexModal and GitInfoMaterial chore: update types for configMap to use SwitchItemValues enum
|



Description
This pull request refactors the way expiration dates are handled for API tokens, moving from using
momentand custom types to a more standardized approach withdayjsand shared types from the common library. The changes improve type safety, maintainability, and UI consistency for selecting and displaying expiration dates.Expiration date handling and UI improvements:
momentand custom date types withdayjsand standardized types (ExpirationDateSelectOptionType) throughout API token components, includingCreateAPIToken,EditAPIToken, andRegenerateModal. [1] [2] [3] [4] [5] [6] [7]DateTimePickercomponent and improved option handling with typed values, enhancing the UI and code reliability. [1] [2] [3]DATE_TIME_FORMATSfrom the common library, ensuring consistent display of dates. [1] [2] [3] [4] [5]Dependency cleanup:
react-datesand its type definitions frompackage.json, as date handling is now done via shared components and libraries. [1] [2]Fixes https://github.com/devtron-labs/sprint-tasks/issues/2714
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: