-
Notifications
You must be signed in to change notification settings - Fork 12
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 messaging #56
base: trunk
Are you sure you want to change the base?
Conversation
Thoughts on this? |
I like that it no longer says test theme because that was confusing. I found a report that was empty: "Found on" is confusing to me because the result has not been presented yet. Front-end testsTests performed on the active theme with the theme test data installed. Test Name: Page should contain body classResultProblems were found on the following links: "example.com/?p=1241" (via: singular.php) does not contain a body class |
Do you think the links would be more clear if the permalink setting was updated to post name? |
We need help from UX experts who knows the best way to deliver negative messages. 🤔 |
That was a mistake, I deleted that comment.
I think you're correct. I think we can maybe remove the What about: I think talking in terms of template files should make sense for authors? |
Yes I think using template files is easier. Also because it is more similar to how the older tools like theme check and the theme sniffer works. |
"We" is problematic when it is not known who "we" are, But using passive voice should also be avoided. |
f572517
to
96aee1a
Compare
Alright, I made some updates to provide the template instead of the URL when possible. When we don't have access to the template file name we just use the URL. I also added some logic to remove duplicates although it only removes exact duplicates so there are more improvements that we can make in the future. I also think there are endless improvements to messaging (stripping out test information, wording, linking to helpful docs, etc...) but we can work on those as we go. @carolinan @kafleg Any objections to merging this PR?
|
This reverts commit 292b5d8.
8e07b73
to
bc759cd
Compare
The use of "test-theme" is confusing. |
This PR moves most of the logging logic to a custom jest reporter which will allow us to string together reporters and manage the output better. This also groups the errors by test to reduce the redundancy of the messaging. See #27.
Example Output