-
Notifications
You must be signed in to change notification settings - Fork 13.6k
feat(ui-voip): pre-fill media-call widget from desktop telephony deeplink #40751
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
base: develop
Are you sure you want to change the base?
Changes from all commits
eea5bfd
ecde40c
4667ffa
2d663cb
7808b77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@rocket.chat/desktop-api': minor | ||
| --- | ||
|
|
||
| Adds deeplink and shortcut handling for voice calls by integrating with the desktop app API to allow phone links and global shortcuts triggered outside the app to open the voice widget seamlessly. Related to Rocket.Chat.Electron#3325. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,26 @@ export const usePeerAutocomplete = (onSelectPeer: (peerInfo: PeerInfo) => void, | |
| initialData: [], | ||
| }); | ||
|
|
||
| // Reflect an externally-selected phone number (e.g. forwarded from a `tel:`/`callto:` deeplink | ||
| // by the Desktop app) in the visible input. `value` is derived from `userId` only, so a peer | ||
| // set as `{ number }` would otherwise leave the field empty. Fires on `peerInfo` identity change | ||
| // only, so manual typing is preserved and re-selecting the same number is a no-op. | ||
| useEffect(() => { | ||
| if (peerInfo && 'number' in peerInfo) { | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| setFilter(peerInfo.number); | ||
| } | ||
|
jeanfbrito marked this conversation as resolved.
|
||
| }, [peerInfo]); | ||
|
|
||
| // When the dial-pad holds a phone-number peer (e.g. pre-filled from a deeplink), keep the | ||
| // selected peer in sync with manual edits so the call dials the number the user actually sees, | ||
| // not the original one. Status-based peers (`userId`) keep their existing selection. | ||
| const updateNumberFilter = (next: string) => { | ||
| setFilter(next); | ||
| if (peerInfo && 'number' in peerInfo) { | ||
| onSelectPeer({ number: next }); | ||
|
jeanfbrito marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the transfer modal has a number peer selected, clearing the autocomplete now re-selects Useful? React with 👍 / 👎. |
||
| } | ||
| }; | ||
|
|
||
| const status = useUserPresence(peerInfo && 'userId' in peerInfo ? peerInfo.userId : undefined); | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -58,7 +78,7 @@ export const usePeerAutocomplete = (onSelectPeer: (peerInfo: PeerInfo) => void, | |
|
|
||
| return { | ||
| options, | ||
| onChangeFilter: setFilter, | ||
| onChangeFilter: updateNumberFilter, | ||
| onChangeValue: (value: string | string[]) => { | ||
| if (Array.isArray(value)) { | ||
| return; | ||
|
|
@@ -84,6 +104,6 @@ export const usePeerAutocomplete = (onSelectPeer: (peerInfo: PeerInfo) => void, | |
| }, | ||
| value: peerInfo && 'userId' in peerInfo ? peerInfo.userId : undefined, | ||
| filter, | ||
| onKeypadPress: (key: string) => setFilter((filter) => filter + key), | ||
| onKeypadPress: (key: string) => updateNumberFilter(filter + key), | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| import { act, renderHook } from '@testing-library/react'; | ||
|
|
||
| import { useDesktopTelephonyListener } from './useDesktopTelephonyListener'; | ||
| import type { PeerInfo, SessionState } from '../context/definitions'; | ||
|
|
||
| const baseSession = { | ||
| connectionState: 'CONNECTED', | ||
| peerInfo: undefined, | ||
| transferredBy: undefined, | ||
| muted: false, | ||
| held: false, | ||
| remoteMuted: false, | ||
| remoteHeld: false, | ||
| hidden: false, | ||
| supportedFeatures: [], | ||
| } as const; | ||
|
|
||
| const sessionFor = (state: SessionState['state']): SessionState => { | ||
| if (state === 'closed' || state === 'new') { | ||
| return { ...baseSession, state, callId: undefined }; | ||
| } | ||
|
|
||
| return { ...baseSession, state, callId: 'call-id', peerInfo: { number: '000' } } as SessionState; | ||
| }; | ||
|
|
||
| type TelephonyCallback = (payload: { phoneNumber: string; rawUri: string }) => void; | ||
|
|
||
| const setupDesktopBridge = () => { | ||
| let registered: TelephonyCallback | undefined; | ||
| const onTelephonyCallRequested = jest.fn((cb: TelephonyCallback) => { | ||
| registered = cb; | ||
| }); | ||
|
|
||
| Object.defineProperty(window, 'RocketChatDesktop', { | ||
| value: { onTelephonyCallRequested }, | ||
| writable: true, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| return { | ||
| onTelephonyCallRequested, | ||
| fire: (phoneNumber: string) => | ||
| act(() => { | ||
| registered?.({ phoneNumber, rawUri: `tel:${phoneNumber}` }); | ||
| }), | ||
| }; | ||
| }; | ||
|
|
||
| const clearDesktopBridge = () => { | ||
| Object.defineProperty(window, 'RocketChatDesktop', { | ||
| value: undefined, | ||
| writable: true, | ||
| configurable: true, | ||
| }); | ||
| }; | ||
|
|
||
| const renderListener = (initialState: SessionState['state']) => { | ||
| const toggleWidget = jest.fn(); | ||
| const selectPeer = jest.fn(); | ||
| const { rerender } = renderHook( | ||
| ({ state }: { state: SessionState['state'] }) => useDesktopTelephonyListener({ sessionState: sessionFor(state), toggleWidget, selectPeer }), | ||
| { initialProps: { state: initialState } }, | ||
| ); | ||
| return { | ||
| toggleWidget, | ||
| selectPeer, | ||
| setState: (state: SessionState['state']) => act(() => rerender({ state })), | ||
| }; | ||
| }; | ||
|
|
||
| afterEach(() => { | ||
| clearDesktopBridge(); | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('registers a single telephony callback once, at mount', () => { | ||
| const bridge = setupDesktopBridge(); | ||
| renderListener('closed'); | ||
| expect(bridge.onTelephonyCallRequested).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('opens the widget pre-filled when the widget is closed', () => { | ||
| const bridge = setupDesktopBridge(); | ||
| const { toggleWidget, selectPeer } = renderListener('closed'); | ||
|
|
||
| bridge.fire('+15551234567'); | ||
|
|
||
| expect(toggleWidget).toHaveBeenCalledWith<[PeerInfo]>({ number: '+15551234567' }); | ||
| expect(selectPeer).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('sets the number without re-toggling when the widget is already open and idle', () => { | ||
| const bridge = setupDesktopBridge(); | ||
| const { toggleWidget, selectPeer } = renderListener('new'); | ||
|
|
||
| bridge.fire('5551234567'); | ||
|
|
||
| expect(selectPeer).toHaveBeenCalledWith<[PeerInfo]>({ number: '5551234567' }); | ||
| expect(toggleWidget).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it.each(['calling', 'ringing', 'ongoing'] as const)('ignores and drops the request while a call is %s', (state) => { | ||
| const bridge = setupDesktopBridge(); | ||
| const { toggleWidget, selectPeer, setState } = renderListener(state); | ||
|
|
||
| bridge.fire('5551234567'); | ||
|
|
||
| expect(toggleWidget).not.toHaveBeenCalled(); | ||
| expect(selectPeer).not.toHaveBeenCalled(); | ||
|
|
||
| // Dropped, not parked: returning to idle must not re-open the widget with the stale number. | ||
| setState('closed'); | ||
|
|
||
| expect(toggleWidget).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('applies a number delivered before the widget settles into an idle state', () => { | ||
| // Cold-start decouple: the number is delivered (stored) while the session may still be | ||
| // transitioning; the open is driven by the effect once the widget reports an idle state. | ||
| const bridge = setupDesktopBridge(); | ||
| const { toggleWidget } = renderListener('closed'); | ||
|
|
||
| bridge.fire('5551234567'); | ||
|
|
||
| expect(toggleWidget).toHaveBeenCalledWith<[PeerInfo]>({ number: '5551234567' }); | ||
| }); | ||
|
|
||
| it('does not re-apply the number after it has been handled', () => { | ||
| const bridge = setupDesktopBridge(); | ||
| const { toggleWidget, selectPeer, setState } = renderListener('closed'); | ||
|
|
||
| bridge.fire('5551234567'); | ||
| expect(toggleWidget).toHaveBeenCalledTimes(1); | ||
|
|
||
| // The widget opens (state -> 'new'); the pending number is already cleared, so no re-apply. | ||
| setState('new'); | ||
|
|
||
| expect(toggleWidget).toHaveBeenCalledTimes(1); | ||
| expect(selectPeer).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does nothing when the desktop bridge is unavailable', () => { | ||
| clearDesktopBridge(); | ||
| expect(() => renderListener('closed')).not.toThrow(); | ||
| }); | ||
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.
Are the changes to this file really needed? 🤔
Having the number (instead of number and filter) selected seems more inline the way we already pre-fill the widget when opening via DM rooms.
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.
I considered the chip-only approach, but I think the changes are needed — number peers and userId peers have different display contracts in the current widget:
filter == selected numberis already the invariant for number peers ondevelop. When a user dials manually (types a number, picks the first option),onChangeValueselects{ number }but doesn't clear the filter — so the input keeps showing the selected number. The sync effect just extends that same invariant to numbers selected externally (deeplink), instead of leaving the input empty only in that one path.userIdpeer — a different shape. There the chip carries the display (name/avatar) and the input is a search box for finding someone else. For a number peer the input is the dial surface; an empty input with the number only in the chip would make the number un-editable in place (wrong extension, missing prefix), which is a common need fortel:links.updateNumberFilteralso fixes a bug that exists ondeveloptoday, independent of deeplinks: type123→ select the first option → type4→ input shows1234but Call dials123. Keeping the peer in sync with edits closes that stale-dial gap for the manual flow too.Happy to walk through it if you'd like — but dropping the sync would reintroduce the empty-input inconsistency and the stale-dial edit bug for number peers.