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

Improve displaying validation errors' invalid markup in Gutenberg block warning notice #3664

Closed
westonruter opened this issue Oct 29, 2019 · 4 comments · Fixed by #4401
Closed
Labels
P0 High priority
Projects
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Oct 29, 2019

Feature description

The compatibility tool now includes nice translatable error messages instead of error codes:

Screen Shot 2019-10-29 at 12 56 25

This has not yet fully been done for the error listed in the warning notice at the block level:

Screen Shot 2019-10-29 at 12 56 11

Essentially the logic from \AMP_Validation_Error_Taxonomy::get_error_title_from_code() and \AMP_Validation_Error_Taxonomy::filter_manage_custom_columns() needs to be adapted from PHP to be included in JS.

The text style should be consistent as well.

Aside: It would be great if more information could also be included in the warning without requiring the user to navigation to the Validated URL screen. The user should be able to manage whether or not the invalid markup is Removed or Kept straight from the warning itself, and have access to all of the context. This is part of a larger issue: #2316.


The validation error warning should also include:

  • Plugin source information.
  • Deep link to validated URL screen with the validation error expanded.

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

Acceptance criteria

  1. Improve the message on the validated URL screen
  2. Current logic in JS should be in PHP and included in validation errors response.
  3. Ensure the error messages are consistent

Implementation brief

QA testing instructions

Demo

Changelog entry

@jauyong
Copy link

jauyong commented Nov 26, 2019

Created #3821 and #3822 and removed it from this issue

@westonruter westonruter moved this from In Progress to To Do in Ongoing Jan 12, 2020
@westonruter westonruter moved this from To Do to Backlog in Ongoing Jan 16, 2020
@westonruter westonruter self-assigned this Jan 16, 2020
@westonruter
Copy link
Member Author

westonruter commented Jan 17, 2020

With the work being done to add stylesheet information to the validated URL screen (#4026), we also need to surface this information proactively in the post editor, in a similar way to how validation errors will get surfaced. One idea would be to incorporate the admin bar item similar to what Site Kit does.

The same admin bar item could also be present on the frontend. Clicking it would trigger a validation request and then show the results right there on the frontend instead of having to go to the validated URL screen.

@kmyram kmyram added the P0 High priority label Feb 6, 2020
@kmyram kmyram moved this from Backlog to To Do in Ongoing Mar 3, 2020
@westonruter westonruter modified the milestones: v1.5, v1.6 Mar 17, 2020
@westonruter
Copy link
Member Author

westonruter commented Mar 17, 2020

Bumping this to v1.6.

@westonruter westonruter moved this from To Do to In Progress in Ongoing Mar 17, 2020
@westonruter westonruter modified the milestones: v1.6, v1.5 Mar 17, 2020
@westonruter westonruter moved this from In Progress to Ready for Review in Ongoing Mar 17, 2020
@westonruter
Copy link
Member Author

Fixed by #4401.

@westonruter westonruter moved this from Ready for Review to Ready for QA in Ongoing Mar 18, 2020
@westonruter westonruter moved this from Ready for QA to Done in Ongoing Apr 3, 2020
@westonruter westonruter removed their assignment Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority
Projects
Ongoing
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants