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 timezone and datetime formatting on workspace member level #5699

Closed

Conversation

AdityaPimpalkar
Copy link
Contributor

closes: #5140

Copy link

github-actions bot commented May 31, 2024

TODOs/FIXMEs:

  • // TODO: use the user's saved time zone. If undefined, default it with the user's detected time zone.: packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarDisplaySettings.tsx
  • // TODO: use the user's saved date format.: packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarDisplaySettings.tsx
  • // TODO: use the user's saved time format.: packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarDisplaySettings.tsx

Generated by 🚫 dangerJS against 73034ac

@AdityaPimpalkar AdityaPimpalkar marked this pull request as ready for review June 2, 2024 23:14
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added timezone and datetime formatting fields to WorkspaceMember type in GraphQL
  • Introduced DateTimeSettings component for user-specific datetime preferences
  • Refactored and moved timezone and datetime utilities to workspace-member module
  • Deleted redundant components and constants related to datetime settings
  • Updated mock data to include new timezone and datetime formatting properties

Copy link
Contributor

@martmull martmull left a comment

Choose a reason for hiding this comment

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

Hey, added some first comments, think we should use enums for workspace-member entity as you suggest, also we would like to create a graphql type to be able to use those enum values in the front

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Introduced timezone and datetime formatting at the workspace member level
  • Updated GraphQL queries and mutations to use new DateFormat and TimeFormat enums
  • Enhanced DateTimeSettings.tsx for granular control over date and time settings
  • Added new fields and enums for timeZone, dateFormat, and timeFormat in workspace-member.dto.ts
  • Updated workspace-member.workspace-entity.ts to define and use new enums for date and time formats

@@ -0,0 +1,242 @@
import { FieldMetadataComplexOption } from 'src/engine/metadata-modules/field-metadata/dtos/options.input';

export enum WorkspaceMemberLocaleEnum {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry but I don't think we'll realistically want to cover these any day. I thought we could do something simpler, no?

Cf Discord message:
Screenshot 2024-07-16 at 10 56 00

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@lucasbordeau
Copy link
Contributor

This PR commit history is dirty and there is a lot of changes here that we don't need on locale and colorScheme.

I create a new PR and handpicked the relevant changes : #6408

@FelixMalfait
Copy link
Member

@lucasbordeau I don't really get your comment to be honest, the commit history get squashed. And as for the changes on locale and colorScheme this was an ask I made to @AdityaPimpalkar during review to remain consistent and avoid creating debt by having those inconsistencies between different fields. He did it because I asked

@FelixMalfait
Copy link
Member

What was the issue with the PR? The vision is for WorkspaceMember is to become an object visible in the UI at somepoint, we're trying to migrate everything to standard objects (like the work we're doing on Note/Task). The work done by @AdityaPimpalkar was in line with that long-term vision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date and time formatting and timezone
8 participants