[Modal] Fix aria-hidden restore logic

[mackersD]

• I have followed (at least) the PR section of the contributing guide.

Closes #15837

The logic that hides/shows nodes via the aria-hidden attribute will now exclude nodes that were already hidden before the modal was mounted.

[mui-pr-bot]

Details of bundle changes.

Comparing: ac39205...6609918

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	-0.05%	-0.04%	317,872	317,710	87,513	87,477
@material-ui/core/Paper	-0.00%	-0.01%	68,279	68,278	20,366	20,364
@material-ui/core/Paper.esm	-0.00%	-0.02%	61,574	61,573	19,162	19,159
@material-ui/core/Popper	0.00%	-0.01%	28,945	28,945	10,399	10,398
@material-ui/core/Textarea	0.00%	+0.04% 🔺	5,513	5,513	2,373	2,374
@material-ui/core/TrapFocus	0.00%	+0.06% 🔺	3,753	3,753	1,577	1,578
@material-ui/core/styles/createMuiTheme	0.00%	-0.02%	16,009	16,009	5,793	5,792
@material-ui/core/useMediaQuery	0.00%	+0.09% 🔺	2,597	2,597	1,100	1,101
@material-ui/lab	-0.00%	-0.01%	140,036	140,035	43,403	43,400
@material-ui/styles	-0.00%	+0.01% 🔺	51,699	51,698	15,346	15,348
@material-ui/system	0.00%	-0.05%	15,420	15,420	4,393	4,391
Button	-0.00%	-0.02%	84,297	84,296	25,723	25,717
Modal	+2.27% 🔺	-0.22%	14,107	14,427	5,103	5,092
Portal	0.00%	-0.06%	3,473	3,473	1,572	1,571
Slider	-0.00%	-0.02%	74,702	74,701	23,253	23,248
colorManipulator	0.00%	0.00%	3,904	3,904	1,544	1,544
docs.landing	0.00%	0.00%	55,119	55,119	13,940	13,940
docs.main	-0.06%	-0.00%	648,864	648,478	204,532	204,523
packages/material-ui/build/umd/material-ui.production.min.js	-0.01%	-0.03%	291,191	291,153	83,354	83,328

Generated by 🚫 dangerJS against 6609918

[oliviertassinari]

I'm doing a deep dive into the logic. I'm migrating the Modal from the classes API to hooks. Once it's done. I will come back to this pull request :).

[mackersD]

@oliviertassinari Thanks for keeping me posted.

[oliviertassinari]

@mackersD You can follow #16254. In theory, we could work in the two in parallel as they touch different files. But I would be interested in finding ways to make the logic correct as well as bundle size efficient.

[oliviertassinari]

I'm looking at the web accessibility guidelines, it seems that the aria-hidden approach we are using is considered "legacy": https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html. They encourage the aria-modal="true" instead.
However, from the UI components, I can benchmark, it seems that there is no consensus on what's the correct approach is.

@eps1lon Do you have a preference?

[eps1lon]

Sounds expected. WAI-ARIA considers it legacy and most of the libraries haven't caught up.

On the other hand I only have access to chrome vox which isn't considered a good screen reader as far as I remember. It will still read content from stacked modals and the body in the WAI-ARIA practices. I don't get this behavior for our SimpleModal demo.

But this is only for chrome vox. We should verify that other screen readers work recognize aria-modal as well and make the rest of the page inert. aria-modal would be convenient but if it's not supported by many screenreaders we should stick to the aria-hidden workaround.

[oliviertassinari]

@eps1lon Thanks for the input. I'm in favor of keeping the current aria-hidden logic waiting for aria-modal to be better supported: https://developer.paciellogroup.com/blog/2018/06/the-current-state-of-modal-dialog-accessibility/.

I have refactored the ModalManager code, hopefully, it's simpler.

[oliviertassinari]

@mackersD It's a great first pull request on Material-UI 👌🏻. Thank you for working on it!

[mackersD]

😁 thanks!