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

feat: Add idle detection to collaboration feature #2877

Merged
merged 19 commits into from
Feb 4, 2021
Merged

Conversation

tomayac
Copy link
Member

@tomayac tomayac commented Jan 29, 2021

📢 PLEASE SEE #2877 (comment) for the description of this PR, the below is no longer accurate.

The problem

When collaborating with others, it can at times be really confusing: Is the other person just staring at the screen thinking? Are they busy in another app? Did they grab a coffee ☕️ and their screen has even locked?

The solution

The Idle Detection API is a new Web platform API proposal that lets Web applications know if the user is present at their device. This API notifies developers when a user is idle, indicating such things as lack of interaction with the keyboard, mouse, screen, activation of a screensaver, locking of the screen, or moving to a different screen. A developer-defined threshold triggers the notification.

How it works

This PR adds Idle Detection to the collaboration feature. It works as follows:

  1. When initiating a collaborative session, there is now a new checkbox that lets the user opt in for their activity to be shared:

Screen Shot 2021-01-29 at 15 12 41

  1. If the user checks the checkbox, a permission prompt pops up, where the user can allow or block the API:

Screen Shot 2021-01-29 at 15 12 58

  1. If the user blocks the API, nothing happens. If the user allows the API, Excalidraw will then share the user's state with all collaborators as follows:
  • The user is active:

    Screen Shot 2021-01-29 at 15 16 14
  • The user is idle, but their screen is still unlocked:

    Screen Shot 2021-01-29 at 15 17 54
  • The user is idle, and their screen has locked:

    Screen Shot 2021-01-29 at 15 18 42
  1. Should the user at any stage decide to no longer want to share their activity status, they can stop sharing by unchecking the checkbox in the collaboration dialog:

Screen Shot 2021-01-29 at 15 19 02

Important background info

The minimum threshold for idle detection to kick in is 60s. That is, when you lock the screen or become inactive, it can take up to one minute for the change to be propagated. This is so different sites cannot collaborate and fingerprint you based on your idle state (more details).

Testing this feature

To test this, enable the chrome://flags/#enable-experimental-web-platform-features flag. Be sure to set a username, else, it won't show up. The final deployed version will use an origin trial (similar as we had with the File System Access API). You don't have to wait one minute to test state changes if you use Chrome DevTools:

Screen Shot 2021-01-29 at 16 01 34

@vercel
Copy link

vercel bot commented Jan 29, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/24xh73pu2
✅ Preview: https://excalidraw-git-idle-detection.excalidraw.vercel.app

@tomayac tomayac requested review from dwelle and lipis January 29, 2021 15:27
@dwelle
Copy link
Member

dwelle commented Jan 29, 2021

My first thoughts were: let's not add another checkbox and complicate the UI. But then I read you need to opt-in (makes sense), so we can't make it non-optional :(.

If I understand the API correctly, it won't go into idle state if the user is doing something in other tabs (which for the purpose of the collab session is useless).

If we think this adds enough value, we can easily roll out a custom implem which will have these benefits:

  1. We can define idlestate as documentNotVisibleForSomePeriod OR userDidntInteractForSomePeriod, which is IMO more applicable for our use case.
  2. Won't require opt-in (privacy-wise, this shouldn't be a big deal) → no checkbox.

If we were worried about timers being slowed down in inactive tabs, we can send pingAlives to our WS server (and stop sending them when blurring the document).

@tomayac
Copy link
Member Author

tomayac commented Jan 29, 2021

If I understand the API correctly, it won't go into idle state if the user is doing something in other tabs (which for the purpose of the collab session is useless).

Not just doing something in other tabs (that is, still in the browser), but also doing something in other apps (like looking up technical details in a PDF opened in Preview that they need for a flow diagram you’re collaborating on in Excalidraw). The core use case is knowing the other party is still active as in “using their computing device”, so you know they’re still there.

@dwelle
Copy link
Member

dwelle commented Jan 29, 2021

Not just doing something in other tabs (that is, still in the browser), but also doing something in other apps (like looking up technical details in a PDF opened in Preview that they need for a flow diagram you’re collaborating on in Excalidraw). The core use case is knowing the other party is still active as in “using their computing device”, so you know they’re still there.

Is it useful to know the person is on their device doing some unrelated work? If she's not focusing on the canvas, what good will this knowledge be to her collaborators?

It's also rather privacy intrusive (and being a new API, and given the nature of Excalidraw, we'd need to let users know to what extent they'll be spied on).

@tomayac
Copy link
Member Author

tomayac commented Jan 29, 2021

Is it useful to know the person is on their device doing some unrelated work? If she's not focusing on the canvas, what good will this knowledge be to her collaborators?

It can make a difference, as in “do you get a coffee, too, since your collaborators all are gone with their screen locked”, or do you maybe wait a little longer.

It's also rather privacy intrusive (and being a new API, and given the nature of Excalidraw, we'd need to let users know to what extent they'll be spied on).

It’s a progressive enhancement; teams (or individuals) can agree to turn this on or leave it off. We have seen similar UI for years in essentially all installed chat clients that have an option to set your status automatically, starting from good ol’ ICQ, to Skype, to Miranda, to Adium (that’s the last I had installed since everything is on the Web now). Here’s an example documentation page from Adium or another from Zendesk chat.

@dwelle
Copy link
Member

dwelle commented Jan 29, 2021

We must answer the following questions:

  1. Usefulness.

    It can make a difference, as in “do you get a coffee, too, since your collaborators all are gone with their screen locked”, or do you maybe wait a little longer.

    This is just one use case. It may be useful in a chat app, but in case of a Excalidraw collaboration session where you usually work on something in bursts and then disconnect, the above use case is limited.

    A counter example (in case of long collab session) is that you switch to other work (e.g. coding or doing other unrelated work) and you want to signal you're NOT looking at the canvas — something the idle-detection API does not address at all.

    Unlike in chat apps, you can't message people — there's no notification system (and likely won't be) in Excalidraw. If someone isn't looking at the canvas, they may as well be AFK.

  2. Privacy.

    As stated above. If you switch tabs/apps, I'd argue (for Excalidraw) the most useful thing to do is to signal you're away. Anything other will potentially impinge on user privacy.

    I can easily imagine businesses forcing you (or peer-pressuring you) to enable the setting even if you don't want to.

    At the very minimum we'd have to give disclaimers as to the extent of what this opt-in will do.

    Again, I must stress out that one of Excalidraw's key philosophies is 1) privacy, and 2) minimum distractions & intuitive and simple UI.

Anything that's opt-in, and has a privacy disclaimer and considerations, introduces a big friction point, and must justify its cost.

@tomayac
Copy link
Member Author

tomayac commented Feb 2, 2021

Here's a design iteration (not implemented, just sketched as an idea) to make the UI simpler, and more transparent. (The images for the legend would be properly aligned and cropped to show just the cursor of course).

Screen Shot 2021-02-02 at 11 16 16

Screen Shot 2021-02-02 at 11 16 37

Thanks, @dwelle, for your thoughts already. Any other ideas from anyone else?

@vjeux
Copy link
Contributor

vjeux commented Feb 2, 2021

I feel like the fact that this feature is opt-in from Chrome makes it unappealing within Excalidraw. If everyone has it, then this is a nice to have feature and would be helpful to know who is looking at the document. But I don't believe that this is worth prompting people to enable. Adding a hidden option is likely not going to get any usage and add a lot of complexity to the codebase.

I believe we should not add this. Thanks @tomayac for bringing it up, would have been a nice thing!

@tomayac
Copy link
Member Author

tomayac commented Feb 2, 2021

Gotcha, and fair point about the prompt. That’s a good signal to feed back to Chrome engineering too.

I can easily refactor this PR to not use the new API, but a more traditional approach of just looking at input events etc. Would this be appealing then?

@vjeux
Copy link
Contributor

vjeux commented Feb 2, 2021

@tomayac I'm fine displaying the user as active or inactive whether they are currently on the tab, I think there's tab visibility for that. And we shouldn't have any options, just enable it by default.

@tomayac
Copy link
Member Author

tomayac commented Feb 2, 2021

👍 Sounds good. I’ll refactor the PR accordingly.

@dwelle
Copy link
Member

dwelle commented Feb 2, 2021

Gotcha, and fair point about the prompt. That’s a good signal to feed back to Chrome engineering too.

As for feedback on the idle detection API, I think it should support customizing the detection scheme. It should also allow to just specify "detect if user has the tab focused, and is interacting with the document". In that scenario it shouldn't prompt (there's no privacy concern because you can already implement that on your own). It should definitely keep prompting for what it does right now: detecting idle state across tabs & OS apps — not only that, it should explicitly say in the prompt that it'll do this (which I didn't know about when I read it at first).

@tomayac
Copy link
Member Author

tomayac commented Feb 2, 2021

@dwelle This is great feedback! Would you be willing to share this word for word on https://github.com/WICG/idle-detection/issues maybe?

src/locales/bg-BG.json Outdated Show resolved Hide resolved
src/locales/ar-SA.json Outdated Show resolved Hide resolved
@tomayac
Copy link
Member Author

tomayac commented Feb 3, 2021

@dwelle, @vjeux, @lipis Please take another look. This PR has now been refactored to only take page visibility and pointer movement into account for detecting if a user is idle.

@lipis Thanks for having cherrypicked the translation fixes!

src/types.ts Outdated Show resolved Hide resolved
@tomayac
Copy link
Member Author

tomayac commented Feb 4, 2021

We now have three states:

  • 🟢 Active, and in the app.
  • 💤 Idle, but still in the app.
  • ⚫️ Away (shown half transparent), and moved away from the app.

@tomayac
Copy link
Member Author

tomayac commented Feb 4, 2021

@lipis
Copy link
Member

lipis commented Feb 4, 2021

We can iterate later and maybe adjusting the icons according to the actual color of the user.. but something for another PR.

Copy link
Member

@lipis lipis left a comment

Choose a reason for hiding this comment

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

For the first iterration is pretty good! Like I mentioned before, we could tweak the icons/colors later.

Copy link
Member

@excalibot excalibot left a comment

Choose a reason for hiding this comment

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

💯

@tomayac tomayac merged commit 1837147 into master Feb 4, 2021
@tomayac tomayac deleted the idle-detection branch February 4, 2021 10:55
lipis added a commit that referenced this pull request Feb 4, 2021
* 'master' of github.com:excalidraw/excalidraw:
  fix: make scrollbars draggable when offsets are set (#2916)
  chore: Run actions on pull requests as well (#2917)
  feat: Add idle detection to collaboration feature (#2877)
  fix: pointer-events being disabled on free-draw (#2912)
  feat: don't store to LS during collab (#2909)
lipis added a commit that referenced this pull request Feb 4, 2021
* 'master' of github.com:excalidraw/excalidraw: (58 commits)
  chore: Update translations from Crowdin (#2906)
  fix: toolbar unnecessarily eats too much width (#2924)
  fix: mistakenly hardcoding scale (#2925)
  fix: text editor not visible in dark mode (#2920)
  feat: support supplying custom scale when exporting canvas (#2904)
  fix: incorrect z-index of text editor (#2914)
  feat: Show version in the stats dialog (#2908)
  fix: make scrollbars draggable when offsets are set (#2916)
  chore: Run actions on pull requests as well (#2917)
  feat: Add idle detection to collaboration feature (#2877)
  fix: pointer-events being disabled on free-draw (#2912)
  feat: don't store to LS during collab (#2909)
  chore: Update translations from Crowdin (#2898)
  fix: track zenmode and grid mode usage (#2900)
  refactor: Use the latest vercel configuration instead of now (#2893)
  chore: Update translations from Crowdin (#2894)
  Update i18n.ts
  Update locales-coverage-description.js
  feat: add view mode in Excalidraw (#2840)
  chore(deps): bump @sentry/integrations from 6.0.1 to 6.0.3 (#2889)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants