[ModalUnstyled] Fix behavior of not respecting props ariaHidden value

[tech-meppem]

Fix for Modal ariaHidden prop as stated in issue #32029.

Fixes #32039

It is using props.ariaHidden directly because there was a eslint error putting it in the const {} = props above.

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

[mui-bot]

Details of bundle changes

Generated by 🚫 dangerJS against a28f6a7

[tech-meppem]

After further investigation, that simple fix is not enough. I am continuing down the rabbit hole of ModalManager.

Fixed in ModalManager too.

[mnajdova]

Changes look good, let's add test to ensure it won't break again in the future :)

[mnajdova]

@tech-meppem could you add tests for the fix?

[tech-meppem]

@tech-meppem could you add tests for the fix?

Hi
I am now working on the test, sorry for the delay, I don't have much time to work on them.
But I have encountered the same problem as I did with my other PR's, with the re-rendering issue.

This is the code I added:

  it('forwards the ariaHidden prop', () => {
    const elementRef = React.createRef();
    // by default, aria-hidden == !open
    // so test the inverses of that
    const { setProps } = render(
      <ModalUnstyled
        open
        aria-hidden
        ref={elementRef}
        keepMounted
        data-testid="modal"
      >
        <div />
      </ModalUnstyled>,
    );

    const { current: element } = elementRef;
    expect(element.getAttribute('aria-hidden'), 'true when modal open').to.equal('true');

    setProps({
      open: false,
      "aria-hidden": false,
    });

    const elementPost = screen.getByTestId('modal');
    expect(elementPost.getAttribute('aria-hidden'), 'null when modal closed').to.equal(null);
  });

However, a problem occurs in that when I change the props with setProps, the HTML dom has not been re-rendered before I do the next check.
Would it just be better to do one of these, or split them out into 2 separate tests?

[mnajdova]

Take a look at the Modal's test for patterns on how to write the tests.

In this specific case, I would just write two different tests, it would even be more readable.

[tech-meppem]

Take a look at the Modal's test for patterns on how to write the tests.

In this specific case, I would just write two different tests, it would even be more readable.

I've updated it now, and have added 4 tests, for all 4 use cases (open/close + set via props/not set via props).

[mnajdova]

Looks great, thanks for spending time on it :)