Skip to content

Conversation

@judicaelandria
Copy link
Contributor

This PR fix the button trigger loading state
closes #5959

@vercel
Copy link

vercel bot commented Sep 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) Visit Preview Sep 18, 2023 4:43pm

@nx-cloud
Copy link

nx-cloud bot commented Sep 14, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1403984. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 14, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1403984:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

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

if I now click trigger loading, it doesn't change to restore loading either.

not sure why, maybe that state isn't what we're actually looking for 🤷

@ardeora might be able to help here. Also, we're still missing the tests for the devtools from v4. @ardeora I could start adding them for the react adapter, but I think most tests should actually be for the agnostic query-devtools package, right? So I'd need to see one or two examples to get this going because I've never written a test in solid 😬

@judicaelandria
Copy link
Contributor Author

hmm that's weird, I'll have a closer look again to see if I can find the real state

@judicaelandria
Copy link
Contributor Author

@TkDodo I think it should work as expected now

Copy link
Contributor

@ardeora ardeora left a comment

Choose a reason for hiding this comment

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

This is looking good! I can see the expected behavior here. One thing that could also be addressed in this PR maybe that when Trigger loading is clicked. We should disable Trigger Error button? Ideally we should be able to trigger an Error while in loading state, but currently seems way more complex to achieve this

@ardeora
Copy link
Contributor

ardeora commented Sep 18, 2023

if I now click trigger loading, it doesn't change to restore loading either.

not sure why, maybe that state isn't what we're actually looking for 🤷

@ardeora might be able to help here. Also, we're still missing the tests for the devtools from v4. @ardeora I could start adding them for the react adapter, but I think most tests should actually be for the agnostic query-devtools package, right? So I'd need to see one or two examples to get this going because I've never written a test in solid 😬

I have been trying to add the tests. The only issue currently I'm stuck on is we use the ResizeObserver API to correctly add breakpoints to split the query details view 50/50 on smaller widths. The ResizeObserver API currently doesn't exist in jsdom so that causes the tests to error. Trying to find a workaround to that.

@judicaelandria
Copy link
Contributor Author

We should disable Trigger Error button? Ideally we should be able to trigger an Error while in loading state, but currently seems way more complex to achieve this

yes, I'll disable to the button while in loading state

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 18, 2023

looks good, but please also disable the invalidate and reset button, as they will also trigger a refetch. clicking them will make the restore loading not work

@judicaelandria
Copy link
Contributor Author

@TkDodo fixed

@TkDodo TkDodo merged commit 511f247 into TanStack:beta Sep 18, 2023
@judicaelandria judicaelandria deleted the feat/devtools-trigger-loading branch September 19, 2023 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants