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

Workflow phone number field #9324

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Workflow phone number field #9324

merged 2 commits into from
Jan 2, 2025

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Jan 2, 2025

Add phone number field

Capture d’écran 2025-01-02 à 18 10 06

@thomtrp thomtrp force-pushed the tt-workflow-phone-number-field branch from b5849e4 to 3c7228e Compare January 2, 2025 16:58
Copy link
Contributor

@Devessier Devessier left a comment

Choose a reason for hiding this comment

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

It works perfectly. Great job!

@@ -125,6 +128,8 @@ export const FormNumberFieldInput = ({
/>
) : null}
</FormFieldInputRowContainer>

{hint ? <FormFieldHint>{hint}</FormFieldHint> : null}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

@thomtrp thomtrp force-pushed the tt-workflow-phone-number-field branch from 3c7228e to 6ea4c2d Compare January 2, 2025 17:38
@thomtrp thomtrp marked this pull request as ready for review January 2, 2025 17:47
@thomtrp thomtrp enabled auto-merge (squash) January 2, 2025 17:47
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

This PR adds comprehensive phone number field functionality with country code selection and proper formatting support.

  • Added new FormPhoneFieldInput component in /packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormPhoneFieldInput.tsx combining country code selection and phone number input
  • Added FormCountryCodeSelectInput component with flag icons and calling codes display in /packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormCountryCodeSelectInput.tsx
  • Added FormFieldHint component in /packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormFieldHint.tsx for displaying helper text below form fields
  • Modified Tag component in /packages/twenty-ui/src/display/tag/components/Tag.tsx to support preventPadding for better form field integration
  • Added Storybook stories for testing and demonstrating the new phone field components

11 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

onPersist({
primaryPhoneCountryCode: newCountry,
primaryPhoneCallingCode: newCallingCode,
primaryPhoneNumber: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: resetting phone number on country change could lead to data loss - consider preserving it

Comment on lines +35 to +41
const handleNumberChange = (number: string | number | null) => {
onPersist({
primaryPhoneCountryCode: defaultValue?.primaryPhoneCountryCode || '',
primaryPhoneCallingCode: defaultValue?.primaryPhoneCallingCode || '',
primaryPhoneNumber: number ? `${number}` : '',
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

style: handleNumberChange should validate phone number format before persisting

Comment on lines +24 to +28
export const Default: Story = {
args: {
label: 'Phone',
defaultValue: defaultPhoneValue,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Missing required onPersist prop in story args. This will cause runtime errors since FormPhoneFieldInput requires it.

@thomtrp thomtrp merged commit 759dcfa into main Jan 2, 2025
23 checks passed
@thomtrp thomtrp deleted the tt-workflow-phone-number-field branch January 2, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants