-
Notifications
You must be signed in to change notification settings - Fork 470
Add getRoles
and logRoles
utilities
#282
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
Comments
We could extend <form id="container">
<input type="radio" />
<input type="radio" />
</form> const container = document.getElementById('container');
debugDOM(container) // simple use-case
// prints this:
// <form id="container">
// <input type="radio" />
// <input type="radio" />
// </form>
debugDOM(container, { showRoles: true }) // new configuration parameter to print container with ARIA roles
// prints this:
// <form id="container" role="form">
// <input type="radio" role="radio" />
// <input type="radio" role="radio" />
// </form> |
Implements requirements of testing-library#282 Added getRoles and logRoles to a new file, role-helpers.js. Moved buildElementRoleList from src/queries/role.js to src/role-helpers.js This function was also needed in role-helpers, and it didn't seem proper to to expert it from queries/role with a bunch of queries. Makes more sense to be in the helper file i think.
Created a draft PR for this. |
It's a bit misleading to serialize the role as attributes on the actual nodes, IMO. Esp since role is a valid attribute that may already exist. Any alternative representations? |
I feel At the same time, it's true that "mutating" the DOM output could lead to possible misunderstanding. What about |
I'm a fan of |
🎉 This issue has been resolved in version 5.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
* Added npm package lodash.merge Added lodash.merge to handle deep merging in role-helpers. * Add `getRoles` and `logRoles` utilities #282 Implements requirements of testing-library/dom-testing-library#282 Added getRoles and logRoles to a new file, role-helpers.js. Moved buildElementRoleList from src/queries/role.js to src/role-helpers.js This function was also needed in role-helpers, and it didn't seem proper to to expert it from queries/role with a bunch of queries. Makes more sense to be in the helper file i think. * Updated logRole test to not rely on snapshot Thinking it over, using the snapshot didn't make much sense either. So Now this test just checks loosely that the output has the expected labels followed by expected element names * Added role-helpers export to index.js * Changed formatting of logRoles No longer shows child nodes in output cleaerer distinction between groups * Removed some tests that weren't needed * Refactored getRoles and updated related tests Fixed bug in getRoles where deeply nested children could overwrtie each other getRoles is now simlper The tests for getRoles and logRoles now use a more realistic DOM tree * Removed export of buildElementRoleList buildElementRoleList doesn't need to be exported at this time. Previously, src/queries/roles used that function to create elementRoleList variable, but since role-helpers needed that as well it's just created in there and only elementRoleList is exported * Removed lodash.merge dependency Lodash.merge is no longer required with the getRoles refactor * Minor: removed a useless -1 There was a leftover -1 on a calculation that calculates the number of '#'s for the role title in logRoles * Minor: statement no longer had to be temp literal * Reasability / sanity pass * Initial value for reducer switched to array * Fixed bug with node selectors on specificity > 0 nodes Nodes that required greater specificity (ie input[type="radio"] is role radio, not the default textbox) where not getting their roles set correctly. Moved getImplicitAriaRole from src/queries/role to src/role-helpers * Added the role-helpers snapshot * Added basic test coverage for getImplicitAriaRole * Removed un-needed branch Also gives 100% branch coverage * Minor: Added a cleanup afterEach test * Removed unneeded elementRoleMap elementRoleMap was useful before getRoles was refactored, but is no longer being used. * getRoles now handles muultiple implicit roles getRoles no longer assumes one implicit role for a given tag. getImplicitAriaRole was renamed to getImplicitAriaRoles Added a tag (menuitem) that has multiple imlicit roles to test case. * Added npm package jest-serializer-ansi * Snapshot now uses jest-serilizer-ansi Allows us to strip the control characters that debugDOM embeds to change text colors on the console. * Tests now use jest-serilizer-ansi These tests had control characters embedded into their inline snapshots to match output of debugDOM. Now they're stripped out and the tests won't fail if the colors debugDOM embeds changes. * Switched '#' to '-' in the output of logRoles * role-helpers test minor change to reduce LOC * Removed elementRoleList from role-helpers export elementRoleList was previously used by src/queries/role, but is not longer needed since moving getImplicitAriaRoles to src/role-helpers * Made public-api exports from role-helpers spcific Only getRoles and logRoles are exported to the public api * Minor: made testids more consistent * Minor: '\n' handling is more consistent in logRoles * Added TS definitions for getRoles, logRoles * Changed logRoles formatting Switched formatting back to initial proposal to avoid potential problems. * Fixed spelling error Closes #282
* Added npm package lodash.merge Added lodash.merge to handle deep merging in role-helpers. * Add `getRoles` and `logRoles` utilities #282 Implements requirements of testing-library/dom-testing-library#282 Added getRoles and logRoles to a new file, role-helpers.js. Moved buildElementRoleList from src/queries/role.js to src/role-helpers.js This function was also needed in role-helpers, and it didn't seem proper to to expert it from queries/role with a bunch of queries. Makes more sense to be in the helper file i think. * Updated logRole test to not rely on snapshot Thinking it over, using the snapshot didn't make much sense either. So Now this test just checks loosely that the output has the expected labels followed by expected element names * Added role-helpers export to index.js * Changed formatting of logRoles No longer shows child nodes in output cleaerer distinction between groups * Removed some tests that weren't needed * Refactored getRoles and updated related tests Fixed bug in getRoles where deeply nested children could overwrtie each other getRoles is now simlper The tests for getRoles and logRoles now use a more realistic DOM tree * Removed export of buildElementRoleList buildElementRoleList doesn't need to be exported at this time. Previously, src/queries/roles used that function to create elementRoleList variable, but since role-helpers needed that as well it's just created in there and only elementRoleList is exported * Removed lodash.merge dependency Lodash.merge is no longer required with the getRoles refactor * Minor: removed a useless -1 There was a leftover -1 on a calculation that calculates the number of '#'s for the role title in logRoles * Minor: statement no longer had to be temp literal * Reasability / sanity pass * Initial value for reducer switched to array * Fixed bug with node selectors on specificity > 0 nodes Nodes that required greater specificity (ie input[type="radio"] is role radio, not the default textbox) where not getting their roles set correctly. Moved getImplicitAriaRole from src/queries/role to src/role-helpers * Added the role-helpers snapshot * Added basic test coverage for getImplicitAriaRole * Removed un-needed branch Also gives 100% branch coverage * Minor: Added a cleanup afterEach test * Removed unneeded elementRoleMap elementRoleMap was useful before getRoles was refactored, but is no longer being used. * getRoles now handles muultiple implicit roles getRoles no longer assumes one implicit role for a given tag. getImplicitAriaRole was renamed to getImplicitAriaRoles Added a tag (menuitem) that has multiple imlicit roles to test case. * Added npm package jest-serializer-ansi * Snapshot now uses jest-serilizer-ansi Allows us to strip the control characters that debugDOM embeds to change text colors on the console. * Tests now use jest-serilizer-ansi These tests had control characters embedded into their inline snapshots to match output of debugDOM. Now they're stripped out and the tests won't fail if the colors debugDOM embeds changes. * Switched '#' to '-' in the output of logRoles * role-helpers test minor change to reduce LOC * Removed elementRoleList from role-helpers export elementRoleList was previously used by src/queries/role, but is not longer needed since moving getImplicitAriaRoles to src/role-helpers * Made public-api exports from role-helpers spcific Only getRoles and logRoles are exported to the public api * Minor: made testids more consistent * Minor: '\n' handling is more consistent in logRoles * Added TS definitions for getRoles, logRoles * Changed logRoles formatting Switched formatting back to initial proposal to avoid potential problems. * Fixed spelling error Closes #282
Describe the feature you'd like:
With the addition of
aria-query
which made huge improvements to theByRole
queries in #280 (thank you again @szabototo89!) I would like to suggest something which I think would help people discover improved ways to semantically query their output:Suggested implementation:
I'm not sure. Anyone have ideas/want to try to implement this?
Describe alternatives you've considered:
Alternatively people have to learn all the possible roles which is unlikely to happen. Having this will encourage people to use semantic roles to query around the DOM for what they're looking for which would be really helpful.
Also, it occurs to me that it may be even more helpful to log every element on the page with the queries that could be used to find it. That would be AMAZING, but seems like a pretty hard problem...
Teachability, Documentation, Adoption, Migration Strategy:
We'd want to feature this in the docs so people get used to using this regularly in their workflow.
I'm in favor of this (obviously) and would love to see a PR for it. I'm going to be away from the keyboard on a family trip for the next week so I will not be responsive for a while, but I'd love to read what people think/come up with when I get back :)
The text was updated successfully, but these errors were encountered: