Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

Update server clipboard on tab activation #143

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ARTI1208
Copy link
Contributor

No description provided.

@ARTI1208 ARTI1208 requested a review from SerVB March 23, 2022 17:32
@codecov-commenter
Copy link

Codecov Report

Merging #143 (7d409cf) into master (268cf3f) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #143   +/-   ##
=========================================
  Coverage     25.26%   25.26%           
  Complexity       63       63           
=========================================
  Files           132      132           
  Lines          3903     3903           
  Branches        408      408           
=========================================
  Hits            986      986           
  Misses         2896     2896           
  Partials         21       21           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 268cf3f...7d409cf. Read the comment docs.

Copy link
Member

@SerVB SerVB left a comment

Choose a reason for hiding this comment

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

Cool but let's improve documentation a bit.

Safari is not supported due to restrictions on clipboard access.
Also site requires secure context support.

Fallback implementation uses ["paste" listener](https://developer.mozilla.org/en-US/docs/Web/API/Element/paste_event). So clipboard is updated on the server only if you invoke that listener, for example, by hitting Ctrl+V or Ctrl+Shift+V. **If you have an application on the server side with a "paste" button, a click on it can paste outdated information unless the listener wasn't invoked**.
Copy link
Member

Choose a reason for hiding this comment

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

Is it really "fallback"? Looks like it's still always invoked when we paste, so we just update the clipboard without need. It can be improved, let's add todo in code or comment in documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

without need

It is still needed in case we copy on the same page

@@ -59,7 +59,13 @@ There are some limitations with clipboard.

When your clipboard is changed on the client side, the server needs to apply the change on its side.

We implement it on the client side via setting ["paste" listener](https://developer.mozilla.org/en-US/docs/Web/API/Element/paste_event). So clipboard is updated on the server only if you invoke that listener, for example, by hitting Ctrl+V or Ctrl+Shift+V. **If you have an application on the server side with a "paste" button, a click on it can paste outdated information unless the listener wasn't invoked**.
We implement it on the client side by reading clipboard contents when the browser tab is activated, then sending to the server.
This allows a pretty seamless user experience.
Copy link
Member

Choose a reason for hiding this comment

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

It seems we still don't support a scenario when another app changes clipboard without activation of our browser tab. This case is rare (probably some special software) but I think we could mention that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the situation when clipboard is changed in background while user uses projected IDE in active browser tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I assume the silence means yes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants