search

aragonone / product

Product pipeline for Aragon One
https://aragon.one
0 stars 1 forks source link

Loading indicator per app #98

Closed sohkai closed 5 years ago

sohkai commented 5 years ago

Idea

User story

As as user, I want each application to inform me that they're not finished syncing all state from the Ethereum network, so their state may not be accurate yet.

Technical requirements

Implement last block caching, and also a loading indicator.

Summary

By implementing caching and a loading indicator, we provide feedback to the user about the load.

Design

Figma board

sohkai commented 5 years ago

From https://github.com/aragonone/product/issues/27#issuecomment-478006257:

Indicators

Loading indicators inform users to wait for an activity to complete.

The following loading indicators are all indeterminate (visualise an unspecified wait time) and relevant to a different loading or syncing state.

03.png

04.png

Figma file

2color commented 5 years ago

Update

Passing sync state to app

I'm implementing this as part of the app state caching which allows us to tap into the point where all past events have been fetched and the subscription is created for current events in https://github.com/aragon/aragon.js/pull/297.

There are different approaches to passing the sync status to the application:

The first approach is simpler to implement and there's already a working proof of concept (https://github.com/aragon/aragon-apps/commit/a6b9d9d9fa663bb4472cd71fd03cbc0e4ac0f21d).

The main downside of the event based approach is that the sync status will be cached which means that app scripts have to take extra precaution to reset its value upon load. Conversely, this is the case for vault balances in finance that have to be refetched upon load irrespective of the cached values.

Regarding the second approach (initially suggested in https://github.com/aragon/aragon.js/pull/297#discussion_r285333605) – syncStatus method. Right now the aragonAPI is used by apps and app scripts to communicate with the native context. Introducing something like syncStatus would be a deviation from that, insofar as it's used to communicate state between the app script and the app. That introduces some complexity, which might be necessary anyhow in the long term.

This would be really useful later on, when we fetch past blocks incrementally and could notify subscribers that we're past a certain stage

@sohkai What do you mean by fetching past blocks incrementally?

Possible states

@bpierre I had a look at https://github.com/aragon/aragon-apps/pull/861. Essentially you introduce three states:

Based on my current implementation, can I assume that SYNC_STATUS_INITIALIZING is the state the application is in by default until the background script has processed the first SYNC_STATUS_SYNCING event (as per https://github.com/aragon/aragon.js/pull/297/commits/02039c3c98a03bfdb4b0837cba74ef1adbd6dc9b)?

2color commented 5 years ago

After some tests with three different states, I think it might make sense to drop the initial SYNC_STATUS_INITIALIZING. The duration between SYNC_STATUS_INITIALIZING and SYNC_STATUS_SYNCING is so short that it might not be of great value to the user to see it flicker from one to the other. Moreover, it does make the implementation slightly more complex (A boolean vs an enum is much simpler to reason about).

Either way, here's a version based on @bpierres' PR (which used mocked data) for token-manager: https://github.com/aragon/aragon-apps/pull/868

sohkai commented 5 years ago

Right now the aragonAPI is used by apps and app scripts to communicate with the native context. Introducing something like syncStatus would be a deviation from that, insofar as it's used to communicate state between the app script and the app.

Both the triggers and syncStatus APIs would still be communicating with the native context; but the native context would just act as a bridge to listeners of the stream (rather than doing any processing, like it does with the other handlers).

What do you mean by fetching past blocks incrementally?

Right now we pull all the past blocks in one getPastLogs(), even if there are a huge number of blocks to pull. Instead, we could "chunk" up the past blocks into 100k - 1M segments and pull for them individually, to have a faster response rate (as well as allowing for a visual loading indicator).

SYNC_STATUS_INITIALIZING

I think it may sense to drop this for now, as we don't have skeleton screens. The main purpose of this state was to render something between the frontend getting loaded, and it being hydrated with the first flush of data from the cache.

A small pain point right now is the "empty state" that gets shown when syncing. We could hide this state in the apps until we know for sure that there's nothing to display, after all the syncing has been completed, and so SYNC_STATUS_INITIALIZING isn't necessary yet.