-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat: enable removing all links from the field #6185
feat: enable removing all links from the field #6185
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
- Enabled removal of all links, including the primary one (
packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/LinksFieldInput.tsx
) - Introduced
handleDeleteClick
to manage link deletion and dropdown closure (packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/LinksFieldMenuItem.tsx
) - Updated URL validation schema to accept empty strings (
packages/twenty-front/src/utils/validation-schemas/absoluteUrlSchema.ts
)
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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
(updates since last review)
- Replaced error throws with logging statements and continued loop (
packages/twenty-server/src/database/commands/upgrade-version/0-22/0-22-update-boolean-fields-null-default-values-and-null-values.command.ts
)
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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
(updates since last review)
- Removed test case for empty string as invalid URL (
packages/twenty-front/src/utils/validation-schemas/__tests__/absoluteUrlSchema.test.ts
) - Potentially allows empty strings as valid URLs
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
Really great work as always! Thank you!
Failing test is not related to this PR, cf https://github.com/twentyhq/twenty/actions/runs/9857499711/job/27216712049 |
Thanks @rostaklein for your contribution! |
closes twentyhq#6041 - enabled removing all dropdown items including the primary one - primary one can be removed even when it is not the last remaining one from the list, this will set the next item on the list as the new primary one (_idk if it was expected to implement this_) https://github.com/twentyhq/twenty/assets/19856731/405a055d-13de-43f4-b3e8-d6a199bfdf24 --------- Co-authored-by: Félix Malfait <[email protected]>
closes twentyhq#6041 - enabled removing all dropdown items including the primary one - primary one can be removed even when it is not the last remaining one from the list, this will set the next item on the list as the new primary one (_idk if it was expected to implement this_) https://github.com/twentyhq/twenty/assets/19856731/405a055d-13de-43f4-b3e8-d6a199bfdf24 --------- Co-authored-by: Félix Malfait <[email protected]>
closes #6041
Screen.Recording.2024-07-09.at.22.50.24.mov