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

Add more descriptive violation logging #25

Merged
merged 2 commits into from
Jun 11, 2016

Conversation

aliciasedlock
Copy link

@aliciasedlock aliciasedlock commented Jun 2, 2016

Original PR from ember-axe: trentmwillis/ember-axe#7

As we are about to use this add-on in a new application, the other developer and I thought it would be nice to have more descriptive logging in the console rather than sifting through the violations object output.

This both updates the default messages that is displayed inside the browser test runner, along with more precise console logging, rather than dumping the entire violations object flat out.

New test runner messaging

screen shot 2016-05-12 at 8 34 10 pm

#### New console level information

screen shot 2016-06-09 at 11 08 32 am

nodesToString: function(nodes) {
return nodes.map(function(node) {
return node.html;
}).join(', ');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if this string is absurdly long?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. There is potential for it to contain a lot of nodes, so perhaps the intent of having the node list at this level makes it less useful.

I can potentially log how many specific violations of that type are present and instead provide an object or array of the offending elements.

On Jun 2, 2016, 12:30 AM -0400, Nathan [email protected], wrote:

Incontent-for/test-body-footer.html(#25 (comment)):

},>>/**>+ * Takes the violations' nodes param and converts it to>+ * a string that can be displayed with the console error.>+ * @param {Array} nodes>+ * @return {String}>+ */>+ nodesToString: function(nodes) {>+ return nodes.map(function(node) {>+ return node.html;>+ }).join(', ');

What if this string is absurdly long?


You are receiving this because you authored the thread.
Reply to this email directly,view it on GitHub(https://github.com/ember-a11y/ember-a11y-testing/pull/25/files/f15c6f6bb86efa59014b009c10f876c3a0d8d07a#r65482880), ormute the thread(https://github.com/notifications/unsubscribe/AAjYQcgi1ArUeP2C8jfGzrYQVyhy-LiBks5qHlxrgaJpZM4IsIU9).

@aliciasedlock
Copy link
Author

Change has been implemented locally, but I would like to test this on our application prior to marking this as ready.

@aliciasedlock aliciasedlock changed the title Add more descriptive violation logging WIP: Add more descriptive violation logging Jun 8, 2016
@nathanhammond
Copy link
Contributor

Remove the WIP flag and drop another note here when you're calling it ready and I'll merge. 😄 Thanks for your efforts on this!

@aliciasedlock aliciasedlock force-pushed the logging-updates branch 2 times, most recently from 181b37f to 9eeb312 Compare June 9, 2016 14:43
@aliciasedlock aliciasedlock changed the title WIP: Add more descriptive violation logging Add more descriptive violation logging Jun 9, 2016
@aliciasedlock
Copy link
Author

@nathanhammond Second pass done! Screenshot is updated, we now list the number of violations and a separate violations array with the specific nodes in it, rather than converting them to a string.

@nathanhammond nathanhammond merged commit c2c1d7a into ember-a11y:master Jun 11, 2016
@nathanhammond
Copy link
Contributor

Thank you @aliciasedlock!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants