freedomofpress / securedrop-client

a Qt-based GUI for SecureDrop journalists 📰🗞️
GNU Affero General Public License v3.0
41 stars 39 forks source link

"Last Refresh:" should only show up if user is offline or experiencing network issues #747

Open eloquence opened 4 years ago

eloquence commented 4 years ago

As we are switching to a continuous sync (#739), it makes no sense to show "Last Refresh: just now" to the user all the time. Instead, the intended behavior is for "Last Refresh" to only be shown when the user is purposefully offline, or temporarily experiencing connection issues. See the Zeplin screens "p1" and "p2" here.

Note: As shown in the Zeplin screens, the top bar should be rendered in offline mode during a network error. This can be handled as part of a PR to resolve this issue, or separately.

ninavizz commented 4 years ago

Thx for filing this, Erik! Yep, this is a bug from how the spec intended the implementation to work.

When the bar shows the offline icon when there are connection issues, then it is ok to show Last Refresh... however, currently, when there are network issues my bar does not show the dark/offline state.

redshiftzero commented 4 years ago

let's discuss this one during UX sync tomorrow (before implementation)

sssoleileraaa commented 4 years ago

ah, yes, after we decided to do continuous syncs, which was a recent decision, we decided that it would make sense to change the spec/requirements to only show the last_sync timestamp when the user is offline/ the queue is paused because of network issues. the idea is that the client should always be in sync with continuous background syncs unless it can't connect to the server.

it might also be useful to show the last_sync timestamp when the client actually needs to update to match recent changes from the server- i'll think more on this and be ready to chat more tomorrow during our UX sync

ninavizz commented 4 years ago

My own TL;DR on this is that I don't want to confuse users by showing the last updated: 10 minutes ago bit some of the time, but not the rest of the time. That, in and of itself, will confuse users more than deliver them value in certain contexts.

Only showing it when the client is offline via a sucky connection, or in Offline Mode, is where I'd prefer it to be once we're doing syncs every 15sec (or so). As the client is performing on my workstation, today (just updated an hour ago) it's very confusing.

sssoleileraaa commented 4 years ago

it might also be useful to show the last_sync timestamp when the client actually needs to update to match recent changes from the server- i'll think more on this and be ready to chat more tomorrow during our UX sync

thought about it more and i think it only makes sense to show timestamp while offline or experiencing network issues

during our ux meeting we discussed how we only want to show the timestamp in that "activity" area next to the sync icon for now. we don't want to show "downloading messages" or anything else, so, for example, we'll want to remove this logic that shows a message when there are new messages to download:

https://github.com/freedomofpress/securedrop-client/blob/fb745f218935559d2e703a4303f429bc1750dd81/securedrop_client/logic.py#L548-L549

We may add these status messages, or something like them, back after https://github.com/freedomofpress/securedrop-client/issues/468 has been addressed and we are able to inform the user that the downloads for a sync have finished.

Also, one more detail for whomever works on this issue, to get started you'll want to remove the call to update_sync here and only start this timer when offline: https://github.com/freedomofpress/securedrop-client/blob/master/securedrop_client/logic.py#L226.

ntoll commented 4 years ago

OK... so @redshiftzero pointed me at this and I've read through this and related tickets (#775). It feels like this and #775 are saying the opposite things. Happy to do whatever folks want, but an unambiguous story would be required.

eloquence commented 4 years ago

I'm not sure where you see the contradiction @ntoll but this may be easier to discuss synchronously.

sssoleileraaa commented 4 years ago

I think the ambiguity is that this issue says we only want to use the activity status for the last-refresh timestamp, and to remove "Downloading new messages" whereas the other issue #775 says to add "Retrieving new messages" to the activity status.

So let's ignore my comment about what we decided during the UX meeting to removing "Downloading new messages" and "Downloading file"- this can be addressed in #775

eloquence commented 4 years ago

That makes sense. :) For the sake of clarity, the current "Downloading messages" does not satisfy the criteria of #775 yet, as we want the message to be displaying during the entire retrieval operation (along with the animation) for all messages and replies, without flickering on and off. Right now it only displays once, briefly. Regarding the wording, Nina has argued (I think correctly) that we should avoid the word "Downloading" here to avoid confusion with file downloads, hence the proposal to use "Retrieving" instead.

ntoll commented 4 years ago

:+1:

ntoll commented 4 years ago

Heh... just to be clear. This is a symptom of my "numptiness" and certainly not a reflection on your commentary or specification. Just asking for clarity. ;-) Now I have it I'll move forward on this if it's not already been started by Monday.

eloquence commented 4 years ago

What remains to be done here:

I'm taking this off the beta milestone in favor of other polish issues for now.

zenmonkeykstop commented 8 months ago

Adding network label - we'll revisit UX around user notifications about network issues and syncing scheds as part of the proxyv2 client updates.