Avanced Settings: Custom API names for Select & Multi-Select Keys #7489

gitstart-app · 2024-10-08T10:40:52Z

Description

text input was changed because it renders an empty div as the right icon, but the margin and padding affect the layout
we have duplicated code on ExpandedWidthAnimationVariants.ts, because of an eslint rule that prevents more than one const definition, can we ignore the rule?

Demo

https://www.loom.com/share/17a37bf5549a4a23ba12343b6046ec6b?sid=4cf297f3-66db-44c9-9a9b-7bde89b90d02

Refs

#6146

greptile-apps

PR Summary

This PR implements advanced settings for Select and Multi-Select fields in the data model settings, focusing on custom API names for options. Key changes include:

Added 'Advanced Mode' toggle to display and edit API keys for Select/Multi-Select options
Introduced new UI components for advanced settings, including animated transitions
Updated TextInputV2 component to improve right icon and error state handling
Implemented state management using Recoil for advanced mode toggle
Created new animation constants and variants for smooth UI transitions

_{5 file(s) reviewed, 5 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-10-08T10:41:52Z

packages/twenty-front/src/modules/settings/constants/ExpandedWidthAnimationVariants.ts

+    width: 0,
+    overflow: 'hidden',
+    transition: {
+      opactity: { duration: ADVANCED_SETTINGS_ANIMATION_DURATION.opacity },


syntax: 'opactity' is misspelled. It should be 'opacity'.

greptile-apps · 2024-10-08T10:41:53Z

packages/twenty-front/src/modules/settings/constants/ExpandedWidthAnimationVariants.ts

@@ -0,0 +1,31 @@
+import { ADVANCED_SETTINGS_ANIMATION_DURATION } from '@/settings/constants/AdvancedSettingsAnimationDurations';
+
+export const EXPANDED_WIDTH_ANIMATION_VARIANTS = {


style: Consider using a loop or helper function to reduce repetition in the variant definitions.

greptile-apps · 2024-10-08T10:42:22Z

...ules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectForm.tsx

@@ -205,7 +245,33 @@ export const SettingsDataModelFieldSelectForm = ({
        render={({ field: { onChange, value: options } }) => (
          <>
            <StyledContainer>
-              <StyledLabel>Options</StyledLabel>
+              <StyledLabelContainer>
+                <AnimatePresence>


style: This AnimatePresence block could be extracted into a separate component for better readability and maintainability

greptile-apps · 2024-10-08T10:42:23Z

...ules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectForm.tsx

                onDragEnd={(result) => handleDragEnd(options, result, onChange)}
                draggableItems={


style: Consider memoizing this callback to prevent unnecessary re-renders

greptile-apps · 2024-10-08T10:43:18Z

packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx

 >`
  background-color: ${({ theme }) => theme.background.transparent.lighter};
  border: 1px solid
    ${({ theme, error }) =>
      error ? theme.border.color.danger : theme.border.color.medium};
  border-bottom-left-radius: ${({ theme, LeftIcon }) =>
    !LeftIcon && theme.border.radius.sm};
-  border-right: none;
+  border-right: ${({ RightIcon }) => RightIcon && 'none'};


style: Consider using a ternary operator for consistency with other style properties

Bonapara · 2024-10-09T08:54:06Z

Hi! A few feedbacks:

There is a gap between the grip and the field, and the right side of the input lacks a border-radius.

No border radius on the right side + button is primary instead of Tertiary

Let's use :

API keys (lowercase for keys) and let's keep options (no switch to "values")

ijreilly · 2024-10-09T16:41:31Z

...ules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectForm.tsx

+  color: ${({ theme }) => theme.font.color.light};
+  font-size: ${({ theme }) => theme.font.size.xs};
+  font-weight: ${({ theme }) => theme.font.weight.semiBold};
+  margin-bottom: 6px;


can you use theme sizes?

ijreilly · 2024-10-09T20:01:01Z

...ings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectFormOptionRow.tsx

@@ -126,7 +156,10 @@ export const SettingsDataModelFieldSelectFormOptionRow = ({
          onChange({
            ...option,
            label,
-            value: getOptionValueFromLabel(label),
+            value:
+              option.value === getOptionValueFromLabel(option.label)


Can you explain your condition rationale?
Why not use isAdvancedModeEnabled here to decide whether or not label update should trigger value update?

Hello @ijreilly
Before the changes, if the user edited the label, the API value changed accordingly, right? now we are maintaining this synchronization if the user does not change the API value, so the user do not need to write everything again for each API value, if the user just wants to edit few values

Got it thanks, I will add an intermediary const to make that clear

Bonapara · 2024-10-10T08:26:29Z

Margin top is 20px (16+4px) instead of 16px

Otherwise looks good to me!

gitstart-twenty · 2024-10-10T13:28:44Z

Hello @Bonapara
by looking into the Figma we have 20px of gap (16+4px)

Bonapara · 2024-10-10T16:22:42Z

Sorry my bad @gitstart-twenty!

github-actions · 2024-10-11T08:35:31Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 83C7:383CF:4333D39:81EF445:6708E351.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Agitstart-app%5Bbot%5D%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Fri, 11 Oct 2024 08:35:29 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'83C7:383CF:4333D39:81EF445:6708E351'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1728635789'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 83C7:383CF:4333D39:81EF445:6708E351.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Agitstart-app%5Bbot%5D%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-b7c53fdf.json

Generated by 🚫 dangerJS against 23d6d1d

Avanced Settings: Custom API names for Select & Multi-Select Keys

547dee6

greptile-apps bot reviewed Oct 8, 2024

View reviewed changes

Avanced Settings: Custom API names for Select & Multi-Select Keys

6c1b75f

gitstart-app bot changed the title ~~TWNTY-6146 - Avanced Settings: Custom API names for Select & Multi-Select Keys~~ Avanced Settings: Custom API names for Select & Multi-Select Keys Oct 8, 2024

fix typo

02c870d

gitstart-twenty added the -PR: awaiting review label Oct 8, 2024

gitstart-twenty requested a review from ijreilly October 8, 2024 11:27

ijreilly self-assigned this Oct 8, 2024

lucasbordeau mentioned this pull request Oct 8, 2024

Feat: Allow defining option key also in custom select options #7019

Closed

Merge branch 'main' of github.com:twentyhq/twenty into TWNTY-6146

f92740b

ijreilly reviewed Oct 9, 2024

View reviewed changes

ijreilly added -PR: awaiting author and removed -PR: awaiting review labels Oct 9, 2024

gitstart-twenty added 2 commits October 9, 2024 21:03

Merge branch 'main' into TWNTY-6146

3e48441

update UI spacing

da79806

gitstart-twenty added -PR: awaiting review and removed -PR: awaiting author labels Oct 9, 2024

gitstart-twenty requested a review from ijreilly October 9, 2024 21:28

ijreilly added 2 commits October 10, 2024 19:47

add intermediary const to clarify behaviour

afa9e51

Merge branch 'TWNTY-6146' of github.com:twentyhq/twenty into TWNTY-6146

23d6d1d

ijreilly approved these changes Oct 11, 2024

View reviewed changes

ijreilly merged commit 7ceaa87 into main Oct 11, 2024
13 checks passed

ijreilly deleted the TWNTY-6146 branch October 11, 2024 08:34

ijreilly mentioned this pull request Oct 17, 2024

Avanced Settings: Custom API names for Select & Multi-Select Keys #6146

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avanced Settings: Custom API names for Select & Multi-Select Keys #7489

Avanced Settings: Custom API names for Select & Multi-Select Keys #7489

gitstart-app bot commented Oct 8, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

Bonapara commented Oct 9, 2024

ijreilly Oct 9, 2024

ijreilly Oct 9, 2024

gitstart-twenty Oct 9, 2024

ijreilly Oct 10, 2024

Bonapara commented Oct 10, 2024

gitstart-twenty commented Oct 10, 2024

Bonapara commented Oct 10, 2024

github-actions bot commented Oct 11, 2024

		@@ -0,0 +1,31 @@
		import { ADVANCED_SETTINGS_ANIMATION_DURATION } from '@/settings/constants/AdvancedSettingsAnimationDurations';

		export const EXPANDED_WIDTH_ANIMATION_VARIANTS = {

		onDragEnd={(result) => handleDragEnd(options, result, onChange)}
		draggableItems={

Avanced Settings: Custom API names for Select & Multi-Select Keys #7489

Avanced Settings: Custom API names for Select & Multi-Select Keys #7489

Conversation

gitstart-app bot commented Oct 8, 2024 • edited Loading

Description

Demo

Refs

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

Bonapara commented Oct 9, 2024

ijreilly Oct 9, 2024

Choose a reason for hiding this comment

ijreilly Oct 9, 2024

Choose a reason for hiding this comment

gitstart-twenty Oct 9, 2024

Choose a reason for hiding this comment

ijreilly Oct 10, 2024

Choose a reason for hiding this comment

Bonapara commented Oct 10, 2024

gitstart-twenty commented Oct 10, 2024

Bonapara commented Oct 10, 2024

github-actions bot commented Oct 11, 2024

Log

gitstart-app bot commented Oct 8, 2024 •

edited

Loading