mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

Loading indicators aren't show when switching between landing pages #10637

Closed tofumatt closed 7 years ago

tofumatt commented 7 years ago

Sorry I didn't catch this when doing local testing, I must not have noticed as I was on a fast enough connection that blinking would hide the fact that no loading bars appeared.

Describe the problem and steps to reproduce it:

If you are on a landing page (/extensions/ or /themes/) then navigate to the other landing page, loading indicators are not shown and the old content remains until the new content is loaded from the server.

What happened?

The old content remains until the new content loads.

What did you expect to happen?

There to be an intermediate loading state while the route changes but the load is taking place.

Screencasts

2017-08-01 23 06 15

Loading animation happens on first landing page load, but not on subsequent loads:

2017-08-01 23 06 35

willdurand commented 7 years ago

Hi! I started to read the code related to this issue. It is the loading state attribute that decides whether to display the loading indicators or not, which is only updated when there is an API call.

Question is: how to approach this? loading should maybe computed somewhere else? Maybe by first having a component state attribute and componentWillReceiveProps() with setState() in it. Otherwise not sure what setViewContext() exactly is yet, but maybe this could be there?

kumar303 commented 7 years ago

Yeah, the loading attribute controls the loading indicators (the blurry lines) but also the presence of each set of add-ons, i.e. state.featured. I think the thing to do is reset each set of add-ons when a new request gets started. For example:

diff --git a/src/amo/reducers/landing.js b/src/amo/reducers/landing.js
index 6c159823..7fa7272b 100644
--- a/src/amo/reducers/landing.js
+++ b/src/amo/reducers/landing.js
@@ -18,7 +18,7 @@ export default function landing(state = initialState, action) {
   switch (action.type) {
     case LANDING_GET:
       return {
-        ...state,
+        ...initialState,
         addonType: payload.addonType,
         loading: true,
         resultsLoaded: false,
tofumatt commented 7 years ago

Yup, that solution should be fine.

willdurand commented 7 years ago

Indeed, that does the trick. Thanks for the solution.

ValentinaPC commented 7 years ago

Verified this on AMO-dev FF54(Win 7). Loading indicators are shown almost everywhere. While loading Extensions homepage coming from Explore the indicators are not shown. Things seem to work as expected while coming back from Themes. Is this ok, @muffinresearch ? Please see video: loading-indic