-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Moved Select Options to External Files #11400
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
Moved Select Options to External Files #11400
Conversation
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.
PR Summary
This PR refactors multiple select options to external constants for improved reusability and consistency across the codebase, while also fixing incorrect useLingui imports.
- packages/twenty-front/src/modules/settings/data-model/fields/forms/boolean/components/SettingsDataModelFieldBooleanForm.tsx: Updated to use
getBooleanDataModelSelectOptions(t)
. - packages/twenty-front/src/modules/settings/data-model/fields/forms/components/text/SettingsDataModelFieldTextForm.tsx: Refactored to import external text select options.
- packages/twenty-front/src/modules/settings/data-model/fields/forms/number/components/SettingsDataModelFieldNumberForm.tsx: Now uses external number select options.
- packages/twenty-front/src/modules/settings/playground/components/PlaygroundSetupForm.tsx: Noted potential mismatch with external constant values.
- packages/twenty-front/src/modules/views/view-picker/components/ViewPickerContentCreateMode.tsx: Standardized select options usage with external constants.
💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!
20 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
.../src/modules/settings/admin-panel/health-status/constants/WorkerQueueMetricsSelectOptions.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/settings/playground/constants/PlaygroundSetupSelectOptions.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/settings/playground/components/PlaygroundSetupForm.tsx
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,14 @@ | |||
import { ViewType, viewTypeIconMapping } from '@/views/types/ViewType'; | |||
|
|||
export const getViewPickerTypeSelectOptions = (t: (key: TemplateStringsArray) => string) => [ |
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.
style: Consider adding an explicit return type for getViewPickerTypeSelectOptions for improved type safety.
@@ -0,0 +1,15 @@ | |||
import { QueueMetricsTimeRange } from '~/generated/graphql'; | |||
|
|||
export const getWorkerQueueMetricsSelectOptions = (t: (key: TemplateStringsArray) => string) => [ |
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.
why do we pass t as a parameter and not use it directly from the constant file?
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.
ok got it, let's use msg instead
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.
Great job 👌
Thanks @StormNinja17 for your contribution! |
The PR #11400 introduced changes to the execution permissions of many executable files. These changes aren't correct and must be reverted. cc. @charlesBochet
This is a minor rework of PR #10738.
I noticed an inconsistency with how Select options are passed as props. Many files use constants stored in external files to pass options props to Select objects. This allows for code reusability. Some files are not passing options in this format.
I modified more files so that they use this method of passing options props. I made changes to:
I also noticed that some of these files were incorrectly using useLingui(), so I fixed the import and usage where needed.