Filter accessibility violations before unmounting #49
Conversation
|
|
||
| let violations = results.violations; | ||
| if (shouldIgnoreViolation) { | ||
| violations = violations.filter(v => !shouldIgnoreViolation(v)); |
There was a problem hiding this comment.
I suppose this may be working due to the overwriting of the violations reference, but perhaps we should make this function return violations instead of result.violations, otherwise it's not super obvious what this if statement does when overwriting the violations variable.
There was a problem hiding this comment.
Yes, the function should return the filtered violations. Otherwise the change doesn't work at all (!)
There was a problem hiding this comment.
As an aside, we currently lack tests for this module which would have made this more obvious.
I spend a few minutes yesterday trying to figure out why the IDs didn't match in hypothesis/client#6620 😅 This is a good catch. |
Change a potentially confusing behavior where components were unmounted before calling the `shouldIgnoreViolation` callback. Querying the rendered tree within the callback, eg. to get the ID of an element to check if a violation relates to it, could fail due to this. Fixes #48.
robertknight
force-pushed
the
filter-violations-while-mounted
branch
from
83d1373 to
2197647
Compare
Change a potentially confusing behavior where components were unmounted before calling the
shouldIgnoreViolationcallback. Querying the rendered tree within the callback, eg. to get the ID of an element to check if a violation relates to it, could fail due to this.Fixes #48.