Red loader at AMP menu, without validation errors

[fellyph]

Bug Description

When the users switch between amp plugin sections, there is a load with a red dot when the user has no errors:

AMP.Settings.amp-beta.WordPress.mp4

Steps to reproduce:

• At standard mode
• Click at Settings at AMP plugin menu and Validated URLs
• Repeat the process until see the loader

Expected Behaviour

If the users has no errors, it should not display the loader on red, because it creates a false alert

Screenshots

No response

PHP Version

7.3.5

Plugin Version

2.2 alpha

AMP plugin template mode

Standard

WordPress Version

5.8.2

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

MacOS

Browser(s) Affected

Chrome 96

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

[maitreyie-chavan]

@delawski please could you take a look at this and if reproducible, see if we can include a fix for the same in 2.2?

[westonruter]

This is because the error count starts out as a spinner until the REST API request returns with the counts to display in the menu items. See #5900. So this is working as expected, but I agree it's not ideal. Should the loading indicator be removed? Or should we show the loading indicator only if on a previous page the counts returned as non-zero? We could use sessionStorage to keep track of whether a non-zero count was returned for a menu item, and if so, show the spinner. Otherwise, if no sessionStorage flag was present or the previous count was zero, then show no loading spinner. This should greatly improve the experience.

[delawski]

I'm working on a slightly different solution.

I'm thinking of using a preload middleware for apiFetch whenever the submenu is opened. This way the number of URLs and errors would be available on page load, so there wouldn't be any loading state needed.

If the AMP menu is not expanded (so for any non-AMP related admin page), we would use the existing loading approach as implemented in #5900. I think it's justified to show a loading spinner on hover.

[westonruter]

The approach taken in #5900 was to use IntersectionObserver to delay fetching the counts until we know that the user will need them. No need to make an additional HTTP request on every admin page load if the counts aren't being displayed. Therefore, I don't think preloading is best for server performance.

[delawski]

In #6770 we preload data only if the AMP submenu is expanded, i.e. only if it's needed on page load.

In #5900 an intersection observer is not even set if the submenu is expanded. The validation counts data is fetched via HTTP request right away. That's when we see the flicker of the loading spinner needlessly.

With #6770 we still use an intersection observer for fetching the data but only if the AMP submenu is collapsed. This way we get the best of both approaches.

I guess it will become clearer once you check out #6770 and try it out live.

[delawski]

I've added a description to #6770 - it's now ready for review.