-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore: Use tanstack query for settings #384
Conversation
<Flex align="center"> | ||
<Spinner mr="2" /> | ||
<Text size="2"> | ||
k6 Studio logs in this screen are updated in real-time. | ||
<Text size="2" color="gray"> | ||
Application logs are updated in real-time. | ||
</Text> | ||
</Flex> |
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.
I made it a bit more subtle, but I'm not quite happy with the spinner here: IMO it gives an indication that something is constantly fetching, even though it's not the case.
@@ -65,23 +60,6 @@ export const SettingsDialog = ({ open, onOpenChange }: SettingsDialogProps) => { | |||
formState: { isDirty, errors }, | |||
} = formMethods | |||
|
|||
const onSubmit = async (data: AppSettings) => { |
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.
using tanstack query is more ergonomic and allows us to reused cached settings across different components
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.
aff57f7
to
5eec434
Compare
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.
Left a suggestion, but otherwise it's legit. 💯
Description
How to Test
Checklist
npm run lint
) and all checks pass.npm test
) and all tests pass.Screenshots (if appropriate):
Related PR(s)/Issue(s)