Skip to content
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

Convert paste overlay to a dialog box #1137

Closed
mtlynch opened this issue Oct 7, 2022 · 3 comments · Fixed by #1603
Closed

Convert paste overlay to a dialog box #1137

mtlynch opened this issue Oct 7, 2022 · 3 comments · Fixed by #1603
Assignees
Labels
enhancement New feature or request medium

Comments

@mtlynch
Copy link
Contributor

mtlynch commented Oct 7, 2022

The clipboard paste was one of TinyPilot's earliest features, and we haven't touched it since. Now, it's inconsistent with the rest of TinyPilot's UI in a confusing way.

The whole overlay behavior is a hack to get around the fact that browsers no longer give applications access to the user's clipboard, so the overlay is actually an input field that the user types into when they hit Ctrl+V.

We should convert the overlay to a standard dialog box with a normal input field that's focused on load. That would eliminate the confusion and inconsistency of the current paste overlay.

This YouTube video is a good example of user confusion around this feature: https://youtu.be/yaBFjescHBo?t=349

@mtlynch mtlynch added the enhancement New feature or request label Oct 7, 2022
@jotaen4tinypilot
Copy link
Contributor

Copying over my comment/proposal from #1346, which was an unintentional duplicate of this issue.


What I found quite unintuitive about the paste overlay when I used it for the first time is that it reacts to the keyboard shortcut for pasting (e.g., “Ctrl+V” or “Meta+V”), and then takes action immediately. That way, I don’t get to review what is send to the target machine, and I also don’t have the option to modify the keystroke sequence beforehand.

An alternative approach to this would be to make the paste feature a dialog with a text area. That would allow me to paste, review, and modify my copy, or even to pre-fabricate it then and there from scratch.

Screenshot 2023-03-31 at 16 07 18

One potential downside is that I’d have to do one extra click after pasting to send the text over. (Which to me wouldn’t be super bad.)

We also need to decide on a good wording and/or help text: it should be clear to the user that we are unable to send text (as in: characters), but we technically send keystrokes – these might result in different characters, depending on the keyboard layouts of the source and target system. So the button label probably shouldn’t be “Send Text”, because that wouldn’t be accurate; however, there might be a better wording than “Send Keystrokes”?

@mtlynch
Copy link
Contributor Author

mtlynch commented Apr 3, 2023

I think we should abstract the implementation details from the user and call it "Paste." That's what we've called it so far, and we haven't heard reports of users getting confused.

If we say "keystrokes," I feel like the user will wonder whether they're supposed to put in newlines or some kind of special metacharacter for <Enter>. I'd rather just say "Paste" and we're responsible for recreating the text on the target device.

@jotaen4tinypilot
Copy link
Contributor

Note: we currently have a bug where the “paste” feature wouldn’t reliably transfer larger amounts of text. In order to fix that issue, we might (or might not) shift the mechanism of how we transmit the keystrokes from the UI to the server. It probably makes sense to wait until that bug fix is through, to avoid unnecessary refactoring work, or other friction.

Another random note regarding the above UI mockup: I’m not sure it makes sense to include a character count below the input text area. On the one hand, such a count might provide helpful feedback in some contexts. On the other hand, characters and keystrokes are not the same thing technically – for example, would we include invisible characters in the count, such as line breaks? Also, counting characters depends on the eventual character encoding, but we can’t determine that.

Therefore, providing such a character count would be a leaky abstraction, that could potentially cause confusion or misconceptions of how the “paste” feature actually works. We could still argue, however, that the count would be correct in (probably) the majority of use-cases, so maybe it’s okay product-wise that the feature doesn’t work in a technically accurate way.

@jdeanwallace jdeanwallace self-assigned this Aug 30, 2023
jdeanwallace added a commit that referenced this issue Aug 31, 2023
Resolves #1137

This PR completely removes the old "Paste" frontend code and replaces it
with a custom `<paste-dialog>` element.



https://github.com/tiny-pilot/tinypilot/assets/6730025/880e650f-b938-4fd6-8417-55c806cd6301



Notes:
1. Once the paste-dialog is open, you can just press the "Enter" key to
actually paste it to the target machine. This is in accordance to our
style guide:

https://github.com/tiny-pilot/tinypilot/blob/273b200d9c49c453e753538393a4712f4755dd44/app/templates/styleguide.html#L260-L262
However, this behavior might be unexpected especially if you're manually
typing text into the textarea and want to add a newline character. With
this in mind, I made sure that you can still add a newline character
using `Shift+Enter`.


<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1603"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants