-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Ensure sharing remains enabled if the initial desktop connection attempt fails #55350
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
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. Would it be possible to write a regression test for that?
Contributor
Author
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. Added. In the end I had to refactor the state, because the hardcoded check for the user agent didn't allow me test this properly. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,8 +53,6 @@ import { KeyboardHandler } from './KeyboardHandler'; | |
| import TopBar from './TopBar'; | ||
| import useDesktopSession, { | ||
| clipboardSharingMessage, | ||
| defaultClipboardSharingState, | ||
| defaultDirectorySharingState, | ||
| directorySharingPossible, | ||
| isSharingClipboard, | ||
| isSharingDirectory, | ||
|
|
@@ -70,6 +68,8 @@ export interface DesktopSessionProps { | |
| clipboardSharingEnabled: boolean; | ||
| directorySharingEnabled: boolean; | ||
| }>; | ||
| /** Determines if the browser client support directory and clipboard sharing. */ | ||
| browserSupportsSharing: boolean; | ||
| /** | ||
| * Injects a custom component that overrides other connection states. | ||
| * Useful for per-session MFA, which differs between Web UI and Connect. | ||
|
|
@@ -92,19 +92,19 @@ export function DesktopSession({ | |
| hasAnotherSession, | ||
| customConnectionState, | ||
| keyboardLayout = 0, | ||
| browserSupportsSharing, | ||
| }: DesktopSessionProps) { | ||
| const { | ||
| directorySharingState, | ||
| setDirectorySharingState, | ||
| onClipboardData, | ||
| sendLocalClipboardToRemote, | ||
| clipboardSharingState, | ||
| setClipboardSharingState, | ||
| clearSharing, | ||
| onShareDirectory, | ||
| alerts, | ||
| onRemoveAlert, | ||
| addAlert, | ||
| } = useDesktopSession(client, aclAttempt); | ||
| } = useDesktopSession(client, aclAttempt, browserSupportsSharing); | ||
|
|
||
| const [tdpConnectionStatus, setTdpConnectionStatus] = | ||
| useState<TdpConnectionStatus>({ status: '' }); | ||
|
|
@@ -145,16 +145,15 @@ export function DesktopSession({ | |
|
|
||
| const handleFatalError = useCallback( | ||
| (error: Error) => { | ||
| setDirectorySharingState(defaultDirectorySharingState); | ||
| setClipboardSharingState(defaultClipboardSharingState); | ||
| clearSharing(); | ||
| setTdpConnectionStatus({ | ||
| status: 'disconnected', | ||
| fromTdpError: error instanceof TdpError, | ||
| message: error.message, | ||
| }); | ||
| initialTdpConnectionSucceeded.current = false; | ||
| }, | ||
| [setClipboardSharingState, setDirectorySharingState] | ||
| [clearSharing] | ||
| ); | ||
| useListener(client.onError, handleFatalError); | ||
|
|
||
|
|
@@ -362,14 +361,7 @@ export function DesktopSession({ | |
| <TopBar | ||
| isConnected={screenState.state === 'canvas-visible'} | ||
| onDisconnect={() => { | ||
| setClipboardSharingState(prevState => ({ | ||
| ...prevState, | ||
| isSharing: false, | ||
| })); | ||
| setDirectorySharingState(prevState => ({ | ||
| ...prevState, | ||
| isSharing: false, | ||
|
Comment on lines
-366
to
-371
Contributor
Author
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. There's no
Member
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. Oh my god I forgot that it's a thing in TypeScript. |
||
| })); | ||
| clearSharing(); | ||
| client.shutdown(); | ||
| }} | ||
| userHost={`${username} on ${desktop}`} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.