Skip to content

fix: update ErrorDialog logic and tests #10111

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

Merged
merged 7 commits into from
Oct 6, 2023
Merged

Conversation

Parkreiner
Copy link
Member

@Parkreiner Parkreiner commented Oct 6, 2023

References #9348. Basically, some prep work for the actual feature. I just figured I'd just get these changes out today.

Had to spend a lot of the day debugging an issue with MUI in React Testing Library. Not sure when it was introduced

Changes

  • Updates the tests for ErrorDialog so that it no longer spits out cryptic messages about unexpected state changes
  • Updates ErrorDialog so that the textbox doesn't start nagging you until the textbox loses focus
Screen.Recording.2023-10-06.at.4.41.05.PM.mov

// DOM aware of the changes

// eslint-disable-next-line testing-library/no-unnecessary-act -- have to make sure state updates don't slip through cracks
return act(() => userEvent.type(inputElement, text));
Copy link
Member Author

@Parkreiner Parkreiner Oct 6, 2023

Choose a reason for hiding this comment

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

To be clear, this issue existed with the previous version of the component, too. I just don't know if this is a recent issue or not – the error message is really big and scary looking. Hard to miss, even with the tests still passing.

Going to see if it's worth opening an issue with MUI and/or RTL

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the first reasonable use case for act I've seen in my career :)

@Parkreiner Parkreiner changed the title feat:workspace redirection fix: update ErrorDialog logic and tests Oct 6, 2023
@Parkreiner Parkreiner requested review from a team and Kira-Pilot and removed request for a team October 6, 2023 20:45
Copy link
Member

@Kira-Pilot Kira-Pilot left a comment

Choose a reason for hiding this comment

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

Thanks for working on this - I bet that test was a pita

@Parkreiner Parkreiner marked this pull request as ready for review October 6, 2023 23:40
@Parkreiner Parkreiner merged commit 38bb854 into main Oct 6, 2023
@Parkreiner Parkreiner deleted the mes/template-deletion branch October 6, 2023 23:40
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants