Reviewing: Rerun query when accepting & un-accepting changes

[ndelangen]

What I did

As per @tmeasday's suggestion: I added a call to rerun() to the onAccept & onUnaccept callbacks making mutation

How to test

• open storybook in repo
• make a change thaty will show up in the UI
• start a build & wait for it to report changes
• accept the change, assert this toggles to the reviewed state fast
• un-accept the change, assert this toggles to unreviewed state fast

📦 Published PR as canary version: 0.0.62--canary.86.a78ce21.0

✨ Test out this PR locally via:

npm install @chromaui/[email protected]
# or 
yarn add @chromaui/[email protected]

[linear]

AP-3637 Accept/unaccept buttons don't immediately update after update

They only update when we've polled for new data. The mutation should trigger a rerun of the query.

[tmeasday]

LGTM. Maybe we should rename the var from rerun?

I tried it out and it sure did work. It does briefly show "Accept" again (I guess while the query is in flight). I guess to fix it we'd need to implement our own state var for isReviewing rather than relying on URQL's mutation state. WDYT?

[ndelangen]

Hmmm.. I'm not sure.. it seems like there should also really be some sort of loading indicator?
@ghengeveld How do you feel we should solve this?

[tmeasday]

it seems like there should also really be some sort of loading indicator?

There is a loading indicator, it appears while the mutation is running. It just doesn't show up while the rerun query is running, so you see "Accept" again for a moment.

I'm happy to move on for now and reconsider these UX nuances later for sure, btw. But to do what we want here I think we'd do something like:

const [isReviewing, setIsReviewing] = useState(false);
onAccepting = async () => {
  try {
    setIsReviewing(true);
    await reviewTest(...)

    await rerun();
  } finally {
    setIsReviewing(false);
  }
}

So not super complicated IMO.

[ndelangen]

@tmeasday when I added rerun() I wondered if I should await it.. and I decided it should not, because rerun (according to types) is not an async function.