Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Add warning dialog for unsupported browsers for directory sharing#1110

Merged
ibeckermayer merged 4 commits intomasterfrom
isaiah/sd-handle-unsupported-browsers
Aug 15, 2022
Merged

Add warning dialog for unsupported browsers for directory sharing#1110
ibeckermayer merged 4 commits intomasterfrom
isaiah/sd-handle-unsupported-browsers

Conversation

@ibeckermayer
Copy link
Copy Markdown

@ibeckermayer ibeckermayer commented Aug 13, 2022

Adds logic for popping up a non-fatal dialog when the user attempts to initiate directory sharing on an unsupported browser.

Requires backport to v9/v10

How to test manually

webapps

  1. Change the value below to true
    enableDirectorySharing: false, // note to reviewers: should be false in any PRs.
  2. run the development server like yarn start-teleport --target=https://ec2-35-171-27-185.compute-1.amazonaws.com/
  3. See https://gravitational.slack.com/archives/C02DQ1C2BMW/p1657807702085829 for a username/password combo

What to expect

Due to an arbitrary choice in the ordering of a particular if-else-if statement, you must be logged in as a user with clipboard access disabled to test this change manually

From a browser which doesn't support the File Access API (Safari/FF):

Go to the top right ... menu and click Share Directory

image

You should see a non-fatal warning dialog

image

begin directory sharing on an unsupported browser.
Comment thread packages/teleport/src/DesktopSession/DesktopSession.tsx Outdated
.catch(() => {
setIsSharingDirectory(false);
});
} catch (e) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not used to seeing a promise's .catch used in conjunction with a standard catch block. Are both of them going to be called in the event of an error?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Are both of them going to be called in the event of an error?

No, what this means is if the Promise returned by window.showDirectoryPicker() is rejected (which happens if the user closes the directory picker without selecting a directory), we setIsSharingDirectory(false);. On the other hand, if the call to window.showDirectoryPicker() throws an error (as happens on browsers where this feature is not implemented, i.e.:Uncaught TypeError: window.showDirectoryPicker is not a function), we setDirectorySharingBrowserErr(true);.

Granted I'm making the assumption that those are the only two situations common enough to distinguish between, and we don't need to implement any finer grained handling within each block. In theory it's possible that there's some other reason that the promise might be rejected, and some other error that could be thrown, in which case the behavior may or may not be what we want.


const [canShareDirectory, setCanShareDirectory] = useState(false);
const [isSharingDirectory, setIsSharingDirectory] = useState(false);
const [directorySharingBrowserErr, setDirectorySharingBrowserErr] =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of 3 booleans, would it make more sense to have a directory sharing state? It could encode all of this info:

  • has permissions to share
  • is actively sharing
  • failed to share due to an error

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Isaiah Becker-Mayer and others added 2 commits August 15, 2022 16:31
@ibeckermayer ibeckermayer requested a review from zmb3 August 15, 2022 21:02
@ibeckermayer ibeckermayer enabled auto-merge (squash) August 15, 2022 21:03
@ibeckermayer ibeckermayer merged commit 3510309 into master Aug 15, 2022
ibeckermayer pushed a commit that referenced this pull request Aug 24, 2022
* `SharedDirectoryInfoResponse` (#996)

* `SharedDirectoryListRequest` (#999)

* `SharedDirectoryListResponse` (#1000)

* `SharedDirectoryReadRequest` (#1003)

* `SharedDirectoryReadResponse` (#1005)

* `SharedDirectoryWriteRequest` (#1007)

* `SharedDirectoryWriteResponse` (#1008)

* Tidy up `sharedDirectoryManager` (#1010)

* `SharedDirectoryMoveRequest` (#1045)

* `SharedDirectoryMoveResponse` (#1074)

* `SharedDirectoryCreateRequest` and `SharedDirectoryCreateResponse` (#1090)

* SharedDirectoryDeleteRequest and SharedDirectoryDeleteResponse (#1096)

* Add warning dialog for unsupported browsers for directory sharing (#1110)

* updates yarn.lock
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants