Redesign: restyle jump to first unread message & rework read marker logic (rebased) #2345
Conversation
with placeholder for message count
Before, UserActivitity emitting actions meant that the user had very recently interaction with their hardware. Now it means they are likely looking at the app. You can attach a timer that is aborted when we think the user stops looking at the page (or hasn't touched their hardware for 2 minutes). This works better than the previous approach for larger timeouts, like the 30s we're about to implement for out-of-view RMs
The only behaviour that should have changed here is that presence is also set to online when switching back to the tab/window. Presence is not set to unavailable when coming back to the window/tab, as that might be a bit invasive, but only when timing out.
Uses Timer & changed UserActivity promise based api
🎉 |
| padding-top: 10px; | ||
| padding-bottom: 10px; | ||
| border-bottom: 1px solid $primary-hairline-color; | ||
| z-index: 1000; |
There was a problem hiding this comment.
might want to double check this doesn't get layered weirdly with widgets or dialogs. I can't remember what their zindex is, but historically we've had problems with layering.
| @@ -654,7 +702,6 @@ var TimelinePanel = React.createClass({ | |||
| if (lastDisplayedIndex === null) { | |||
| return; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I kinda like having these empty lines every so often, but won't protest strongly for them
|
thanks for the explanation over at element-hq/element-web#2322. I guess my only concern is that we deliberately implemented presence to not activate when you happened to scan through the riot tabs/windows, but required you to actually be waving the mouse or keyboard around - given it's quite common on WMs to temporarily focus a window without paying any attention to it at all. HOWEVER, this isn't a big enough deal to merit iterating further, imo; i'm much more interested to get the additional sanity on RMs. Can I suggest that we do drop the on-screen RM dissolve time back to ~1s though? 10s feels unnecessarily long. |
|
@ara4n just played around with adding a delay on user activity started by page visibility/focus, but turns out that the mouse/keyboard interaction is also triggered when just switching tabs, not even hovering over the page, so we already had this behaviour before, so leaving it for now. Parked that experiment on |
element-hq/element-web#7591
Abandoned #2322 in favor of this PR as the commit log is a bit cleaned up here. See also comments there.
The read marker logic is also changed, as https://github.com/vector-im/riot-web/issues/4134 has been a long outstanding issue, and making the feature look different without making it work well didn't make sense.
The read marker is now also updated if it is currently above the viewport:
After the timeout elapses, the read marker is moved below the last event in the viewport when timeout elapses, as before.
The timeout is aborted when the user becomes inactive. The read marker is not touched, and the timer starts counting again once the user is active again. What is considered user activity changed a bit as well: the user is considered active when activating the tab/window, moving the mouse or scrollwheel, or using the keyboard. The user is considered inactive when blurring the window/tab, or 2 minutes have elapsed since the last interaction event was fired. This to prevent the read marker being updated when we know the user is not looking at the page.
Also added a Timer class using promises as the clear/setTimeout's were going to grow a bit unwieldy. Should be reusable for other timeout related usecases.
I'm not doing the badge count for now, as many rooms have gaps in their timeline so we can't give an accurate number of messages since the unread marker. Tried it and it looks confusing.
Parked the work for the badge on the
bwindels/jumptofirstunread-badgecountbranch (also on js-sdk) if we ever want to get back to it.