box-ui-elements icon indicating copy to clipboard operation
box-ui-elements copied to clipboard

fix(activity-thread): fix comment focus bug

Open joshmarnold opened this issue 2 years ago • 3 comments
trafficstars

BaseComment select focus is not behaving correctly when interacting with the dropdown menu options. When navigating from the open dropdown menu to a text input (either a child reply from the same parent comment or a different comment’s child reply), select focus is not properly being applied.

File2023-10-03 at 15 23 46

joshmarnold avatar Oct 05 '23 17:10 joshmarnold

It appears that visual testing will have to be used for testing instead of unit.

Here are the unit methods I attempted:

BaseComment.js: The goal was to verify that a comments list item (li) has the correct class. I initially attempted to achieve this through snapshot tests. Unfortunately, this approach failed because the logic that determines if an element is selected is located in the parent of BaseComment component of. That would be ActivityState.js

ActiveState.js I attempted to render the items but this approach failed because the components required for functionality and verification are nested too deeply within.

joshmarnold avatar Oct 05 '23 17:10 joshmarnold

It appears that visual testing will have to be used for testing instead of unit.

Here are the unit methods I attempted:

BaseComment.js: The goal was to verify that a comments list item (li) has the correct class. I initially attempted to achieve this through snapshot tests. Unfortunately, this approach failed because the logic that determines if an element is selected is located in the parent of BaseComment component of. That would be ActivityState.js

ActiveState.js I attempted to render the items but this approach failed because the components required for functionality and verification are nested too deeply within.

Are you not able to mock and stub the children in ActiveState to render what you need in order to test if your changes work?

greg-in-a-box avatar Oct 13 '23 19:10 greg-in-a-box

@greg-in-a-box It turns out that the issue was with the onFocus event not working correctly. Without that working, I can't test this scenario because it's what sets the selected state

https://github.com/testing-library/user-event/issues/592#issuecomment-816987376

Following up: We decided to test with enzyme instead of RTL.

joshmarnold avatar Oct 13 '23 19:10 joshmarnold