Add loading delay adminhtml

[justinbeaty]

Description (*)

As described in #2419 it can be annoying on a fast connection that the admin dashboard "Please wait" message shows up for very fast AJAX requests.

This PR sets a timeout on showing the loading indicator so that it only shows up after 100ms. If the request is quicker than that, the indicator will not show.

I also removed some Internet Explorer stuff...

Paging @ADDISON74 please test :)

Possible TODO, allow this value to be configurable? I'm not really sure it's needed and would cause some extra complexity because we need to output the variable value in a phtml file somewhere so that we can read it in JS. The default of 100ms is a pretty commonly used debounce value.

Related Pull Requests

Fixed Issues (if relevant)

1. Disable page loading effect #2419

Manual testing scenarios (*)

1. Load some quick operation in the Magento dashboard like filter customers by email foobar@example.com. The indicator should not show up if your server is fast (especially if its local)
2. Load some longer operation in the Magento dashboard like filter customers by name Foo Bar. The indicator should show up if you have a good amount of customers.

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[justinbeaty]

I reverted the Internet Explorer stuff. It was breaking and I will create a new PR to 20.0.

[ADDISON74]

Thank you @justinbeaty. I can confirm that the change in this PR solves the annoying issue in the Backend interface, when even though you have a high-performance and fast server, you always receive flashes over the content of the page that it is waiting. I have sorted columns, quick switches between categories and many others, it is obviously something else. I think that the value of 100 ms is fine, at least in my case it did not appear at all.

As for making the value configurable in the interface, if it is a complex task it is not worth the effort. If in time we find that it is necessary to modify it we can make a PR that increases the value to solve that issues whether it will remain 100 ms or it will be 500 ms, we will see in time.

In any case, now I could get used to the OpenMage theme (if better colors and font), I avoided it only because these waiting were much more visually aggressive than in the Magento theme. Now it is a joy for the eyes, it's a pity that we spent so long with this issue.

[ADDISON74]

I made the change in production. At the value of 100 ms very few sections still display the waiting dial. If I increase the value to 150 ms the number decreases even more. It's a real relief not to see the waiting dial like a flash anymore.

We will certainly not please everyone with the value of 100 ms. If we leave it hardcoded few will know about it. Maybe we should analyze that the value is configurable in the Backend.

I would leave it at 100 ms now for testing.

[justinbeaty]

I made the change in production. At the value of 100 ms very few sections still display the waiting dial. If I increase the value to 150 ms the number decreases even more. It's a real relief not to see the waiting dial like a flash anymore.

We will certainly not please everyone with the value of 100 ms. If we leave it hardcoded few will know about it. Maybe we should analyze that the value is configurable in the Backend.

I would leave it at 100 ms now for testing.

The value shouldn't be too high, but I can look into making it configurable.

One thing to test before merging - in dev environment try to set the value to something like 5000. After clicking something that would make it show up, can you break the site at all by clicking on a second thing? For example hitting a submit button twice.

If that's possible, we can work around it by making the overlay disable things on screen right away, but not visible until the timeout.

[ADDISON74]

In Backend > Dashboard when I click on "Most Viewed Products" tab the waiting dial appears. It is the only one I discovered for 100 ms. I will try your test and let's see what is happening.

I will look in the existing code if a configuration variable is used in any JavaScript file. If I find it, I will analyze the implementation. The value is easy to take to a PHTML file used everywhere in the Backend and from there to be taken over by the script. But maybe there is already a solution to be taken directly?

[justinbeaty]

I will look in the existing code if a configuration variable is used in any JavaScript file. If I find it, I will analyze the implementation. The value is easy to take to a PHTML file used everywhere in the Backend and from there to be taken over by the script. But maybe there is already a solution to be taken directly?

There's no way to output a config variable directly into JS. You are right that there are other adminhtml variables outputted into a phtml file somewhere. I'm just not sure where off the top of my head.

[justinbeaty]

@ADDISON74 I have added the value to the configuration dashboard. It is in: Configuration > Advanced > Admin > Admin Theme. The default is 100ms.

[justinbeaty]

I am testing now to see if I can break anything by clicking other buttons before the loading mask would show up.

[ADDISON74]

Good job. This change really has a visual impact for all Backend users. I will test the changes and tell you my opinion. If you don't add anything and everything is fine I will ask you if I can approve it.

[ADDISON74]

1. The new strings must be entered in the CSV translation file.

2. I would mention in the comment that the default value is 100. If someone changes it even after a period forget how long it was, to know the value immediately. I am also analyzing the wording "the recommend value is 100", I have to take a look at other wordings as I want to keep the Magento format, even if some expressions are not exactly English.
   if someone sets another value and after a period is not satisfied with the results, let him know where he can take it to the end. you can mention "the recommended value is 100."

3. We will analyze together the expressions in English because I want it not to be noticed when we add something. I am used to the interface and even if some expressions are not exactly correct they have that specific smell of Magento.

[justinbeaty]

@ADDISON74 Thanks for the reminder, I forgot about the translations.

Maybe:

Number of milliseconds to delay showing the loading indicator. The default value is "100". Enter an empty value to disable the loading indicator delay.

Adapted from #2308

[justinbeaty]

I changed how the loading mask appears. Basically I was worried about it being possible to do actions on the page when the loading indicator should have prevented any clicks (but was delayed due to this PR).

So now the <div id="loading-mask"> element appears right away, but the loading indicator child <div> is delayed. This prevents any clicks to the page immediately as before.

Despite wanting to be able to do this in pure CSS, I did have to add a new <div> to app/design/adminhtml/default/default/template/page.phtml. This only is for the new Admin theme, the legacy didn't need it. So the worst case scenario if someone is on the new theme and overrided that phtml file, that the darkened background would not show up, but that's okay IMO.

@ADDISON74 sorry to make you test this again, but please pull the latest commits and let me know if all looks okay.

[ADDISON74]

Comments

1. You have chosen the right section in the configuration of the Backend (Admin > Admin Theme). I don't think it could have been anywhere else.

2. "Loading Indicator Timeout" description is perfect.

3. If we take into account PR Addendum to PR 2087 #2308, the last sentence should be formulated like this: "An empty value disables the loading indicator delay." or "An empty value disables the delay."

Question
What happens if there is no value in the input box? Will it be 0? I left it empty, I set the value to 0, the behavior seems to be identical, the loading indicator is no longer displayed. I looked in the code and I did not find this situation analyzed.

[justinbeaty]

Question What happens if there is no value in the input box? Will it be 0? I left it empty, I set the value to 0, the behavior seems to be identical, the loading indicator is no longer displayed. I looked in the code and I did not find this situation analyzed.

Issue I set the value to 5 and the loading indicator is not displayed. I set it to 1, it didn't appear like that either. I doubt my system is that fast, most likely the value is not used and must be double checked. Is it possible that it is not displayed due to "display:none"?

Empty and "0" should be the same thing, a 0 millisecond timeout. Or in other words the old behavior where the indicator shows up immediately.

[ADDISON74]

I checked again and the changes made are correct. I appreciate that you made a validation of the value, to be greater than or equal to zero. Thank you for your work @justinbeaty.

I am waiting for you to make the change in that phrase in the comment. I would choose "An empty value disables the delay", because it is obvious that reference is made to the Loading Indicator.

[justinbeaty]

@ADDISON74 fixed translations.

[fballiano]

test right now, it works perfect although to me 100msec didn't do the trick, I still see the loading splash screen, with 200msec it went away (on my local dev environment on mac+homebrew)

[justinbeaty]

@fballiano we can change it to 200ms, I think it makes @ADDISON74 happier too.

[fballiano]

I'd like that :-)

[justinbeaty]

done @ADDISON74 your reviews was dismissed. You can check changes here.

[github-actions]

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 8297316. ± Comparison against base commit d8c52ea.