-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix(opentrons-ai-client): fix example buttons and clear text after submitting prompt #15299
Conversation
closes closes Auth 434 and AUTH 437
const render = (props: React.ComponentProps<typeof PromptButton>) => { | ||
return renderWithProviders(<PromptButton {...props} />) | ||
} | ||
|
||
let mockSetValue = vi.fn() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mockSetValue = vi.fn() | |
const mockSetValue = vi.fn() |
beforeEach(() => { | ||
props = { | ||
buttonText: 'Reagent Transfer', | ||
} | ||
mockSetValue = vi.fn() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mockSetValue = vi.fn() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
thank you for fixing this!
Overview
closes AUTH-434 and AUTH-437
This PR fixes the example buttons and clears the input prompt text after clicking the submit button.
I also removed the unnecessary jotai state variable that was holding onto prompt button text because the prompt button can access the form directly. This allows us to get rid of an unnecessary useEffect which was just listening for changes to the state variable and setting form state with the updated state variable value.
Also, testing react-hook-form is a massive pain. sometimes we need to mock internals, and sometimes we want to test full integration. I introuced patterns for both in this PR. Though honestly I'm not even convinced we need react hook form. Ideally we can get rid of both that and jotai
Changelog
Review requests
Risk assessment
Low