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

[core] Use describeTreeView for selection tests #12647

Merged
merged 3 commits into from Apr 3, 2024

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Apr 3, 2024

Part of #12433

Most tests are just migration with unified API from existing tests.
I did not remove any test.

I have doubt about some of the current behaviors, which IMHO are not what the ARIA spec describes (clicking on a selected item in single selection does not un-select it, but the specs say that it should "toggle" the selection). But for now I migrated all the tests without touching the behaviors to avoid mixing the two.

@flaviendelangle flaviendelangle self-assigned this Apr 3, 2024
@flaviendelangle flaviendelangle added core Infrastructure work going on behind the scenes component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! labels Apr 3, 2024
@@ -98,9 +92,6 @@ describeTreeView<UseTreeViewExpansionSignature>(
});

fireEvent.click(response.getItemContent('2'));
act(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to remove those when moving the focus from the tree to the item

@@ -270,6 +258,35 @@ describeTreeView<UseTreeViewExpansionSignature>(
});
});

// The `aria-expanded` attribute is used by the `response.isItemExpanded` method.
Copy link
Member Author

Choose a reason for hiding this comment

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

Found those tests while migrating the selection tests

@mui-bot
Copy link

mui-bot commented Apr 3, 2024

Deploy preview: https://deploy-preview-12647--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 4e0084b

@flaviendelangle flaviendelangle changed the title [core] Use describeTreeView for selection tests [core] Use describeTreeView for selection tests Apr 3, 2024
Copy link
Contributor

@noraleonte noraleonte left a comment

Choose a reason for hiding this comment

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

LGTM 👌 🎉

I have doubt about some of the current behaviors, which IMHO are not what the ARIA spec describes (clicking on a selected item in single selection does not un-select it, but the specs say that it should "toggle" the selection)

I agree. Maybe we should create an issue/notion page for these inconsistencies with the ARIA specs and discuss how to handle them

@flaviendelangle
Copy link
Member Author

Maybe we should create an issue/notion page for these inconsistencies with the ARIA specs and discuss how to handle them

Here it is: #12654

@flaviendelangle flaviendelangle merged commit a1975c6 into mui:master Apr 3, 2024
17 checks passed
@flaviendelangle flaviendelangle deleted the selection-test branch April 3, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants