remove user refresh and replace with sync icon #732
Conversation
sssoleileraaa
requested review from
kushaldas and
redshiftzero
as code owners
sssoleileraaa
force-pushed
the
remove-refresh-sync
branch
from
0da80a2 to
4f45b35
Compare
|
rebased on latest master, resolving a conflict with the new sync period being every minute instead of every 5 minutes also removed user refresh button tests and added some for non-user-interactive sync icon this is ready for review |
|
did some clean up around removing user-refresh code and only emitting the |
There was a problem hiding this comment.
Thanks @creviera this looks great!
Performed a visual review of the diff, as well as a functional review:
- Start the client in offline mode and check that icon is offline icon
- Log in and see the metadata sync animation
- After 2 seconds see that it is in the inactive state
- Log out see the icon switches back to offline mode
- The client refreshes periodically
- No AppArmor errors observed (resource folder is included as part of this blob https://github.com/freedomofpress/securedrop-client/blob/master/files/usr.bin.securedrop-client#L21)
Also
| @@ -37,7 +37,7 @@ class MetadataSyncJob(ApiJob): | |||
| ''' | |||
|
|
|||
| def __init__(self, data_dir: str, gpg: GpgHelper) -> None: | |||
| super().__init__() | |||
| super().__init__(remaining_attempts=15) | |||
There was a problem hiding this comment.
Is this value used elsewhere? if so we could use a static variable somewhere
|
@emkll i just responded to your review comment so this is ready for rereview thx |
Description
Closes #687
We now animate the metadata sync and instead of showing an active state for the entire length of the metadata sync we always just show a 2 second animation. Here is recording of the animation (it's subtle) after I changed the background metadata syncs to happen every 6 seconds for demo purposes:
Test Plan