Skip to content
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

Wait until data got changed and refresh widgets before returning to previous screens #99

Merged
merged 13 commits into from
May 30, 2024

Conversation

adzialocha
Copy link
Member

@adzialocha adzialocha commented May 29, 2024

This PR makes sure that users directly see newly created or updated data when they return to the previous screen.

The trick is to wait for the pushed screen to return (after a "pop" event) and look up a global dirty flag (RefreshProvider) if the user has created or updated something. If they did, we force a re-load and re-render of the widget which might need to display this update.

On top we're waiting until the document got actually materialized on the node before we return back to the "All Sightings" screen.

Context

The underlying issue was actually not that we didn't wait enough before data got materialized on the node, but that when calling "pop" on the navigator stack it'll return back to a "cached" state of the widgets. That makes sense for performance reason obviously but we need to force-trigger a refresh to make sure the changes are visible.

There's a way to hook into a "pop" callback from the perspective of the screen which pushed the next view on the stack. We can then trigger a re-render there, how this is done depends a bit on the widget but I've made it work.

On top I've made it more "lazy", aka, not trigger a re-render every time we return from somewhere (also bad UX, you don't want to unnecessarily jump in scroll position after returning). To understand if something has changed I've introduced a global state provider where we can keep track of "dirty" flags.

Todo

  • Refresh All Sightings List after creating a new Sighting
  • Refresh All Sightings List after updating a Sighting
  • Refresh All Species List after changing a Species
  • Refresh All Uses + All Local Name Widgets after changing a sighting
  • Only refresh when data actually has been changed

Closes: #19

@adzialocha
Copy link
Member Author

adzialocha commented May 29, 2024

Looking at this bug here right now, am I'm doing something wrong? Any ideas @sandreae?

{
  status: all_bee_sighting_0020df662f01bd4eed879ebb2128edd3e0b55902f179eeaf8978e58011f96b488717(
    meta: {
      viewId: "00206a52968297bafbd2824218ebe8da3174a432c57083ed81b073f3621d1165f7e5"
    }
  ) {
    totalCount
  }
}

And I receive

{
  "data": null,
  "errors": [
    {
      "message": "internal: not an object",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ]
    }
  ]
}

The error occurs as soon as I add the "meta" field in the arguments.

@sandreae
Copy link
Member

Think you need:

{
  status: all_bee_sighting_0020df662f01bd4eed879ebb2128edd3e0b55902f179eeaf8978e58011f96b488717(
    meta: {
      viewId: { eq: "00206a52968297bafbd2824218ebe8da3174a432c57083ed81b073f3621d1165f7e5" }
    }
  ) {
    totalCount
  }
}

@adzialocha
Copy link
Member Author

Think you need:

{
  status: all_bee_sighting_0020df662f01bd4eed879ebb2128edd3e0b55902f179eeaf8978e58011f96b488717(
    meta: {
      viewId: { eq: "00206a52968297bafbd2824218ebe8da3174a432c57083ed81b073f3621d1165f7e5" }
    }
  ) {
    totalCount
  }
}

Aaaaaaah! Haha, should have read our documentation 🥴 Thank you!

@adzialocha adzialocha changed the title Wait until sighting got created before we return to overview Wait until data got changed and refresh widgets before returning to previous screens May 30, 2024
@adzialocha adzialocha marked this pull request as ready for review May 30, 2024 13:04
@adzialocha adzialocha requested a review from sandreae May 30, 2024 13:04
Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

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

This looks and works great to me. Nice pattern and much better UX to have new sightings showing up straight away.


import 'package:flutter/widgets.dart';

enum RefreshKeys {
Copy link
Member

Choose a reason for hiding this comment

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

Very nice pattern 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope thats irony 😝

Copy link
Member

Choose a reason for hiding this comment

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

I mean, kind of, we needed a fix for it and this isn't a bad one 👍

@adzialocha adzialocha merged commit d7d62c7 into main May 30, 2024
1 check passed
@adzialocha adzialocha deleted the wait-until-ready branch May 30, 2024 15:15
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.

Wait until newly created or updated sighting was materialised before returning to main view
2 participants