Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose validation status digest (in admin bar) when DevTools disabled #5166

Closed
amedina opened this issue Aug 4, 2020 · 10 comments
Closed

Expose validation status digest (in admin bar) when DevTools disabled #5166

amedina opened this issue Aug 4, 2020 · 10 comments
Labels
Enhancement New feature or improvement of an existing one Groomed P2 Low priority Punted Validation WS:UX Work stream for UX/Front-end

Comments

@amedina
Copy link
Member

amedina commented Aug 4, 2020

In 2.0 when dev tools are hidden, the user does not have a way to get any information about the validation status of their pages/site. This is problematic in cases when, for example, some invalid markup is introduced by a plugin, the AMP plugins removes it and there is some misbehavior of the site and the user has no ides what is that coming from.

The solution is to provide a middle point between having full validation tools/information exposure to the user, and having none. When in Reader mode for non-tech-savvy users, the plugin should provide a "digest" of validation status to the user as an indication that there are issues.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@amedina amedina added Enhancement New feature or improvement of an existing one Validation WS:Core Work stream for Plugin core labels Aug 4, 2020
@amedina amedina added this to the v2.1 milestone Aug 4, 2020
@westonruter westonruter modified the milestones: v2.1, v2.0 Aug 4, 2020
@westonruter westonruter self-assigned this Aug 4, 2020
@westonruter
Copy link
Member

Is this digest the same as shown in the admin bar dropdown as added on #4986?

image

@westonruter westonruter modified the milestones: v2.0, v2.1 Aug 5, 2020
@westonruter westonruter removed their assignment Aug 5, 2020
@amedina
Copy link
Member Author

amedina commented Aug 6, 2020

That is more detailed than I had in mind. Right now is zero information. A mid point between that and dev tools on would be something like "Your site has validation issues". And perhaps a notice (in admin screen maybe) to either turn dev tools on or contact the system admin, or disable offending plugins.

@westonruter
Copy link
Member

If the user is an admin, there is no need to turn on dev tools as they can still access the validation screens even with dev tools turned off.

For users who are not admins, then that is where we need to mention something but tell them to ask an admin for help, since they would not be able to access dev tools.

@westonruter
Copy link
Member

In this way it's related to #3821 (comment).

@kmyram kmyram added the Groomed label Aug 18, 2020
@schlessera
Copy link
Collaborator

We should change the way we refer to the change we make to the DevTools. What we need is feedback on validation errors for users, even when DevTools is "hidden". However, adding feedback then would basically create a second, simplified DevTools mode that replaces the one we just hid.

Instead of "hiding" or "disabling" DevTools for non-developers, I think we should rephrase this in the sense that we "add a simplified mode" to DevTools.

So what we have now would be the "advanced mode" of DevTools, and for non-developers, we'd add a stripped-down or simplified version, that only focus on giving the most important bits to the user without overwhelming them.

@westonruter
Copy link
Member

I'm not sure that we need to change how we refer to “disabling DevTools”. The simplified error view isn't really part of DevTools, I don't think. For example, in Chrome you can try going to a website with an bad HTTPS certificate and you get an error displayed on the page. You could also open DevTools to get more details about what the error is (I think), but users don't have to touch DevTools to be told about the error.

So I think what we need to do discontinue the exclusive coupling of validation errors to DevTools being enabled. When DevTools are disabled, we'd show the (minimized) errors but not reveal by default developer-focused technical bits that the user wouldn't understand. Instead of saying there is a validation error for an “Invalid script caused by plugin X” we should rather say there is a “Plugin incompatibility in X”. (See #4668.) Non-technical users could then have the needed information to rectify the problem, by switching to a different plugin or suppressing that plugin on AMP pages. There should then be a way to show the technical details (by going to the validated URL screen) or sending the error details to a technical administrator to investigate.

@westonruter westonruter added WS:UX Work stream for UX/Front-end and removed WS:Core Work stream for Plugin core labels Dec 10, 2020
@westonruter
Copy link
Member

Re-designating this in the UX workstream. As mentioned in #2069 (comment) and as is being done in #5589 (related to #4668), I think the key for non-technical users is to surface the theme/plugins that are causing compatibility problems. That being the case, the digest I think should be a list of these components, in the same way that the block editor will be highlighting the theme/plugins responsible for the errors.

One issue here for showing this information in the admin bar is that the validation error source attribute is not available on normal frontend requests, so we'd have to say something initially like "There are AMP compatibility issues on this page." Clicking that menu item could then initiate a validation request to fetch the validation errors and their sources to then populate the in AMP admin bar menu.

cc @johnwatkins0 @jwold

@jwold
Copy link
Collaborator

jwold commented Dec 14, 2020

Should we keep #5697 a separate issue, or merge the two? That feels like a sub-set of this one.

@westonruter
Copy link
Member

Yes, keep separate. This issue is specifically for exposing the information in the admin bar, whereas that issue is for exposing it in the block editor sidebar.

@westonruter westonruter changed the title Expose validation status digest when DevTools and hidden Expose validation status digest (in admin bar) when DevTools and hidden Dec 14, 2020
@westonruter westonruter changed the title Expose validation status digest (in admin bar) when DevTools and hidden Expose validation status digest (in admin bar) when DevTools disabled Dec 14, 2020
@westonruter westonruter modified the milestones: v2.1, v2.2 Feb 11, 2021
@westonruter
Copy link
Member

This depends on being able to initiate REST API request on the frontend. While this can be done now with the ?amp_validate query parameter, it would also be satisfied by #5552.

Note that the validation details should not be made available if the user does not have access to DevTools. If the user does have access, but they turned off Dev Tools in their user profile, then we can surface that information. Otherwise, non-privileged users would just be able to be told that there are validation issues that an administer needs to review.

For administrators with DevTools turned off, the admin bar item can show a warning sign as it does when DevTools are turned on. However, when DevTools are turned off, instead of "Validate URL" the menu item can just say "There are X validation issues."

In both cases, for users with DevTools enabled and disabled, I think we can then make this submenu item actually have a nested submenu. When that submenu is expanded, we can then show a "Loading" message and fetch the validation information for the current URL. We can then list out the theme/plugins which are causing the validation issues, or list out the validation errors in a similar way to they are presented in the AMP Validation sidebar in the editor now. The key thing for users with DevTools turned off is to identify the theme/plugins causing the problems, and not the problems themselves: highlight the cause not the symptoms.

@westonruter westonruter added the P2 Low priority label Oct 25, 2021
@westonruter westonruter modified the milestones: v2.2, v2.3 Nov 30, 2021
@westonruter westonruter removed this from the v2.3 milestone Apr 14, 2022
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one Groomed P2 Low priority Punted Validation WS:UX Work stream for UX/Front-end
Projects
None yet
Development

No branches or pull requests

5 participants