Page MenuHomeElementl

[dagit] Copy-to-clipboard fallback for unsupported contexts
ClosedPublic

Authored by dish on Jun 28 2021, 10:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 2, 6:04 AM
Unknown Object (File)
Tue, Jan 31, 7:45 AM
Unknown Object (File)
Mon, Jan 16, 2:19 PM
Unknown Object (File)
Thu, Jan 12, 12:53 AM
Unknown Object (File)
Jan 9 2023, 1:39 PM
Unknown Object (File)
Nov 18 2022, 12:19 AM
Unknown Object (File)
Nov 18 2022, 12:19 AM
Unknown Object (File)
Nov 18 2022, 12:19 AM
Subscribers
None

Details

Summary

Resolves #4315.

If a user is serving Dagit from http (non-localhost), the navigator.clipboard value is undefined, so any writeText calls we're making to copy values to the clipboard will error out. (There may be other circumstances where this is the case, so I asked in the GH issue.)

Create a very simple hook to provide callsites with Clipboard | null instead of referencing navigator.clipboard directly. Callers can then decide how to handle the case where the clipboard API is unavailable.

EDIT: The diff now implements a fallback for cases that do not support the Clipboard API.

Test Plan

Load Run page and Schedule page in Dagit with useClipboard returning null. Verify that I'm not given the opportunity to trigger an error.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This looks good to me. I think from a user perspective it'd almost be better to fallback quietly to window.alert(stuffForClipboard) so they can select+copy out of there rather than hiding the buttons. I worry people will think the button was removed and file a ticket complaining about it. Didn't realize this was conditionally unavailable though! Super good to know 👍

This revision is now accepted and ready to land.Jun 30 2021, 4:10 AM
dish retitled this revision from [dagit] Disable copying for unsupported contexts to [dagit] Copy-to-clipboard fallback for unsupported contexts.Jun 30 2021, 3:34 PM
dish edited the summary of this revision. (Show Details)