Clicking a phone number should copy its value #9069
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds clipboard copy functionality when clicking phone numbers, replacing the default phone app navigation behavior with a success notification upon copying.
- Added click handler in
packages/twenty-front/src/modules/ui/field/display/components/PhonesDisplay.tsxto copy phone numbers to clipboard - Click handler only works in focused view, needs to be added to unfocused state for consistency
- Missing error handling notification when clipboard copy fails
- Inconsistent handling of undefined phone numbers could cause runtime errors
- Should consider preserving phone app navigation as secondary action (e.g. ctrl+click)
💡 (5/5) You can turn off certain types of comments like style here!
1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
packages/twenty-front/src/modules/ui/field/display/components/PhonesDisplay.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/field/display/components/PhonesDisplay.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/field/display/components/PhonesDisplay.tsx
Outdated
Show resolved
Hide resolved
| @@ -30,6 +38,29 @@ const StyledContainer = styled.div` | |||
| `; | |||
|
|
|||
| export const PhonesDisplay = ({ value, isFocused }: PhonesDisplayProps) => { | |||
| const { enqueueSnackBar } = useSnackBar(); | |||
There was a problem hiding this comment.
let's not call the snackar in such a low component. Instead we should be able to pass call back to the PhoneDisplay component: onPhoneClick and implement the copy behavior (and snackbar) in PhonesFieldMenuItem
There was a problem hiding this comment.
okay ill update the copy behaviour
There was a problem hiding this comment.
@charlesBochet Hi charles im not quite sure about implementing the snackbar in phonesFieldMenuItem as i saw the action that if we click on the dropdown then the component is summoned but our main purpose is to copy when its clicked on the chip so im quite not sure why do i implement it there
There was a problem hiding this comment.
because the PhoneDisplay is too low level (UI) and should not take dependency on the snackbar.
Instead it could expose a prop: onPhoneNumberClick that would allow the parent to decide what to do with this click event (here copy to the copyboard + display snackbar toast)
There was a problem hiding this comment.
hi @charlesBochet i have edited by passing a prop to phoneDisplay and i have added the functionality to its parent . Can you review it now?
| event.preventDefault(); // Prevent default link behavior | ||
|
|
||
| try { | ||
| if(phoneNumber !== undefined){ |
There was a problem hiding this comment.
is isDefined helper + linter does not seem to be effective (npx nx run twenty-front:lint:ci)
Lucifer4255
force-pushed
the
phone_number_copy
branch
from
37b8bf4 to
0ff225c
Compare
All.-.People.-.Google.Chrome.2024-12-13.23-31-08.mp4
I have added the functionality of copying the phone number to clipboard according to the issue #8905 . If anything needed to change just comment in my PR