-
Notifications
You must be signed in to change notification settings - Fork 15
Add a loading GIF or screen on first render of browserAction popup #93
Comments
This patch makes it possible for the user to undelete a deleted product to their list of tracked products while the browserAction popup remains open. If the browserAction popup closes, any deleted products will be removed permanently. Implementation notes: * Add the DELETE_PRODUCT action which dispatches when the user clicks the ‘X’ button for a product. * The product card is greyed out, cannot be clicked to open the product page, and an Undo button appears in the top right corner. * Add the UNDELETE_PRODUCT action which dispatches when the user clicks the ‘Undo’ button for a product. * The product card is restored to its original view and behavior. * Replace the REMOVE_PRODUCT action with the DELETE_MARKED_PRODUCTS action which dispatches when the browserAction is closed. * Any greyed out cards will be gone the next time the popup is opened. In Nightly, when the DELETE_MARKED_PRODUCTS action is dispatched, the error from Issue #60 is logged, though it does not affect the functionality of this patch. Follow-up issues: * #93: Show a loading screen for the first render before the app's state has been loaded.
Here are some guidelines from bryanbell:
|
I took the liberty of extracting the minimal CSS for the throbber from the browser myself: https://codepen.io/anon/pen/rZqYOW?editors=1100 |
Per this comment in #104 , when this animation is showing, it will supercede (i.e. show over) the "Send Feedback" icon. |
@bryanbell What's the motivation for showing a loading indicator during extraction? Part of #29 will involve attempting extraction earlier in case it succeeds earlier, and re-trying later if it fails. It's unclear to me if we show the loading indicator continuously until the final attempt or if we only show it until the first attempt finishes, pass or fail. I also wonder about the case where extraction fails; the loading indicator (specifically the one in the header) might deter the user from interacting with the UI until it is finished, and if extraction happens to take a long time, they'll be sitting staring at that indicator for a while. If the loading indicator is in the header, what do we show in the listing while we're waiting for the product listing to be loaded? Is it just a blank space, or do we only show the title bar with a loading indicator? My preference would be that we show the loading indicator for the state load only, and show it in place of the listing until the listing is ready to be displayed. |
I'm going to attempt to answer some of your questions based on the chat I had with Bryan about this a while ago; though ultimately Bryan will need to sign off:
I mentioned to him that extraction was also async, and Javaun has expressed concern that it takes a while for the extraction to complete once he navigates to a page, so it seemed like an indicator for that piece would be helpful too. My assumption here is that we show the loading indicator until we succeed or all attempts fail.
If extraction fails, the attempt will have been made and
Bryanbell suggested we leave it blank and set it to the minimum height, which is the empty onboarding popup's height. |
Confirmed with @bryanbell via Slack:
|
The browserAction scripts don’t run until its page exists, which doesn’t happen until the user clicks the browserAction icon to open the popup.
Once the popup is opened, before we can render the list of tracked products, we have to get the list from the extension’s local storage (via
loadStateFromStorage
). This is an async call that could be slow to return, say if the user has saved a lot of products and/or they have a slow internet connection.Even after
loadStateFromStorage
returns, there is additional async work that needs to be done to make the loaded state available asprops
in our top level app component.We therefore don’t want to wait for this async function and all of its follow-on async tasks to return before rendering at least something in the popup.
The text was updated successfully, but these errors were encountered: