search

aimpowered / LibOrate

A videoconferencing companion app developed by AImpower.org to provide emotional and relational support during videoconferencing.
https://liborate-alpha.vercel.app
MIT License
0 stars 2 forks source link

Fix "toggles nametag display" test case #27

Closed blickly closed 8 months ago

blickly commented 8 months ago

Currently, the 'toggles the nametag display on switch change' test in __tests__/nametag.test.tsx seems to have incorrect behavior (and a related TODO). We should try to fix the behaivor and update the test so that it tests that the behavior is how we intend it.

@kexinF, can you take a look?

kexinF commented 8 months ago

Hi @blickly, I spent some time on this part but still have no clue.

We have a unique data-testid for this attribute, and we use getByTestId to retrieve it, ensures we fetch the right element.

I tried both userEvent await userEvent.click(switchInput); and fireEvent await fireEvent.change(switchInput, { target: { 'aria-checked': true } }); to trigger this attribute.

I also tried to check its status using expect(switchInput).toBeChecked();, or directly check the value of 'aria-checked' using expect(switchInput.getAttribute('aria-checked')).toBe('true');, but both did not work.

Can you please help me with this? Thank you!

blickly commented 8 months ago

I don't think we should be using test ids in the tests. Maybe as a first step we can change to using the "role" in the test instead of the test id?

kexinF commented 8 months ago

Yes, I was able to use the 'find by role'. I also modified the test so we are only rendering the nametag part. I hope this will help to narrow down the things a little bit. Please check commit 49bd98d335ed259719464154b5a6496fb34fa887.

kexinF commented 8 months ago

@blickly Just curious, if I did not @ you, will you be notified?

blickly commented 8 months ago

I took a look at this in more detail, and it looks like the current usage of the MUI Switch component doesn't follow conform to it's documented API.

Take a look at the documentation at https://mui.com/material-ui/react-switch/ (and especially the section on Accessibility), which makes several recommendations on how to use the component in an accessible way. I suspect that once using the component correctly, it will also work better with the testing library.

P.S. Looks as the filer of this issue, I get the same notifications whether or not you use the @ notification.

kexinF commented 8 months ago

Solved in the latest PR. Thank you Ben!