Feat/new timezone settings #6398
Conversation
TODOs/FIXMEs:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The pull request introduces new timezone settings and updates various components and tests to support user preferences for date and time formats.
- GraphQL Updates: Added new enums and fields in
packages/twenty-front/src/generated/graphql.tsxto support user preferences. - Component Refactors: Deleted
SettingsAccountsCalendarDateFormatSelect.tsx,SettingsAccountsCalendarTimeFormatSelect.tsx, andSettingsAccountsCalendarTimeZoneSelect.tsx, moving functionality to new components inprofile. - New Components: Added
DateTimeSettings.tsx,DateTimeSettingsDateFormatSelect.tsx,DateTimeSettingsTimeFormatSelect.tsx, andDateTimeSettingsTimeZoneSelect.tsxfor managing date and time settings. - Test Updates: Modified test files to use new enums from GraphQL types, ensuring consistency and type safety.
- Recoil Integration: Updated state management in
SettingsAccountsCalendarDisplaySettings.tsxandDateTimeDisplay.tsxto use Recoil for user settings.
30 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
|
|
||
| const updateWorkspaceMember = async (changedFields: any) => { | ||
| if (!currentWorkspaceMember?.id) { | ||
| throw new Error('User is not logged in'); |
There was a problem hiding this comment.
Style: Throwing an error here could disrupt the user experience. Consider a user-friendly error message or fallback.
| } | ||
| }; | ||
|
|
||
| if (!isDefined(currentWorkspaceMember)) return; |
There was a problem hiding this comment.
Style: Returning undefined here may cause issues. Consider a fallback UI or message.
| timeZone, | ||
| value, | ||
| }: DateTimeSettingsDateFormatSelectProps) => { | ||
| const setTimeZone = timeZone === 'system' ? detectTimeZone() : timeZone; |
There was a problem hiding this comment.
Style: Consider caching the result of detectTimeZone() to avoid redundant calculations.
| label: `${formatInTimeZone( | ||
| Date.now(), | ||
| setTimeZone, | ||
| DateFormat.MONTH_FIRST, |
There was a problem hiding this comment.
Style: Using Date.now() directly in the formatInTimeZone function may cause issues with re-renders. Consider memoizing the formatted date.
|
Closing this because it was a test, the correct PR is #6408 |
New PR for timezone feature