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

Allow Developer Tools to be turned off to minimize (even hide) validation error info #2673

Closed
westonruter opened this issue Jun 21, 2019 · 10 comments · Fixed by #4955
Closed
Assignees
Labels
Changelogged Whether the issue/PR has been added to release notes. Groomed UX Validation WS:UX Work stream for UX/Front-end
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Jun 21, 2019

AMP validation error notices are concerning to non-technical users:

Screen Shot 2019-06-21 at 17 02 31 (1)

Rejection of validation errors should be limited to higher-privileged users, with Authors and Contributors (or other non-Admin role) shown validation errors warnings that just prompt them either to remove the invalid block (#2285) or to escalate the error to an administrator for support.

There should perhaps be a filter to configure the verbosity of the warnings for authors. There's a tough balance here because at one hand the warnings could be hidden to not concern users, but then at the same time the errors mean that markup is being removed. So if the validation error notice is not presented to the user, there needs to be a way for those validation errors to be escalated to an administrator.


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

Acceptance criteria

  • Whether developer tools are disabled should no longer be determined by the template mode. Developer tools should be able to be enabled in Reader mode the same way as they are in Transitional/Standard modes.
  • A new user preference (exposed on the profile screen) should be exposed for whether developer tools are enabled. (This may also be exposed in the wizard per Wizard step: Asking user if they're technical #4705.)
  • Only administrators have access to the developer tools, although this should be gated behind a new amp_validate capability which by default is mapped by the manage_options capability (in the same way as customize is by default granted by edit_theme_options).
  • When developer tools are turned off, the validation screens are not displayed in the admin menu (although they can still be accessed by users who can amp_validate).
  • When developer tools are turned off, synchronous validation is not performed in the editor when saving a post. Instead, saving a published post should schedule an event to validate that URL. (To be done as part of Use WP Cron to periodically check site URLs for AMP validation #1756.)
  • When there are unreviewed validation errors, a Site Health test should be make this known. See Use WP Cron to periodically check site URLs for AMP validation #1756 (comment).

Implementation brief

QA testing instructions

Demo

Changelog entry

@archon810
Copy link

I'd love to see this brilliant feature, combined with a way to run AMP validation async #2069.

@westonruter
Copy link
Member Author

westonruter commented Sep 11, 2019

See also this extension to the AMP plugin: https://gist.github.com/westonruter/31ac0e056b8b1278c98f8a9f548fcc1a

This is a workaround for sites encountering slowness when updating posts due to the synchronous loopback requests which happen at post save to report back validation errors to the editor; instead of running at the save_post action, the post is queued for validation in an immediate event via WP Cron. The AMP plugin intends to make this process asynchronous, but until that lands this plugin can be used. A side effect of turning off synchronous validation is that validation error warnings are no longer displayed in the editor. To see the errors, you have to go to the AMP Validated URLs screen.

@westonruter
Copy link
Member Author

westonruter commented Oct 15, 2019

On a related note, I think the “Enable AMP” checkbox should be only presented to administrator users. Instead of non-admin authors seeing AMP validation errors and just going straight to disabling AMP for the post, they should instead be directed to contact the administrator to get advice on the issue and then the administrator can decide whether to disable AMP for the post. See also #2314.

@westonruter
Copy link
Member Author

Related: #4593

@westonruter
Copy link
Member Author

@amedina amedina changed the title Minimize validation error warnings for non-admin users Minimize validation error warnings when Developer Tools are turned off May 13, 2020
@westonruter
Copy link
Member Author

On a related note, I think the “Enable AMP” checkbox should be only presented to administrator users. Instead of non-admin authors seeing AMP validation errors and just going straight to disabling AMP for the post, they should instead be directed to contact the administrator to get advice on the issue and then the administrator can decide whether to disable AMP for the post.

Furthermore, the “Enable AMP” checkbox should be gated behind whether developer tools are enabled, not just whether the user is an administrator. For more on this, see #1864 (comment).

@westonruter
Copy link
Member Author

The mechanism for disabling dev tools is being explored in #4644, with the code likely being split out from that prototype branch.

@westonruter westonruter changed the title Minimize validation error warnings when Developer Tools are turned off Minimize (even hide) validation error warnings when Developer Tools are turned off May 23, 2020
@kmyram kmyram added the UX label May 26, 2020
@westonruter
Copy link
Member Author

Something else to consider is allowing users who have dev-tools turned off to still access the validated URL screen but in read-only mode.

@westonruter
Copy link
Member Author

Upon thinking about this further, I think we should indeed restrict dev tools to just the users who can manage_options. So there would be three possible states:

  1. Administrator who has developer tools enabled would see validation error messages in the editor and would have access to the validation error screens.
  2. Administrator who has developer tools disabled would see minimized validation error messages, prompting to reach out to a fellow administrator who can address the issue, along with a link to the validated URL screen.
  3. Non-administrator would only see minimized error message with prompt to contact administrator. No access to developer tools would be granted.

The degree (and design) to which validation error messages are minimized is to be determined. To start with we could just hide them altogether, and then at a later point we could re-introduce them in a discrete way. When the validation errors are hidden, then it would be up to surfacing validation errors via Site Health. See #1756 (comment).

@westonruter
Copy link
Member Author

Also, as noted on #4705 (comment):

If it so happens that there are technical and editorial administrators. Since both administrators would have developer tools turned on by default, there needs to be an easy way for non-technical administrators to suppress them. This could be accomplished by having a button in the editor notice to permanently turn off the warnings, rather than just just to dismiss them for the editor session. Maybe clicking the dismiss button in the notification could even open a prompt asking if you want to permanently hide the validation error messages.

@westonruter westonruter changed the title Minimize (even hide) validation error warnings when Developer Tools are turned off Allow Developer Tools to be turned off to minimize (even hide) validation error info Jun 14, 2020
@westonruter westonruter self-assigned this Jun 15, 2020
@kmyram kmyram added the Groomed label Jun 23, 2020
@westonruter westonruter assigned amedina and unassigned westonruter Jul 12, 2020
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jul 17, 2020
@kmyram kmyram added the WS:UX Work stream for UX/Front-end label Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Groomed UX Validation WS:UX Work stream for UX/Front-end
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants