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

Revisit Redux states for measurements #1857

Closed
joverlee521 opened this issue Sep 12, 2024 · 1 comment · Fixed by #1881
Closed

Revisit Redux states for measurements #1857

joverlee521 opened this issue Sep 12, 2024 · 1 comment · Fixed by #1881
Assignees

Comments

@joverlee521
Copy link
Contributor

Follow up to #1848

Redux state for measurements is a bit of a mess right now because of a few of reasons

  1. deferred loading of the measurements JSON
  2. the ranking of state to use (url params > collection display defaults > app defaults)
  3. narrative page changes with the same dataset but changes in URL params

Copying from @jameshadfield comment

Prior to this work, we load and parse the main JSON and read URL queries all in one redux step (largely createStateFromQueryOrJSONs). Sidecar JSONs are loaded via subsequent actions and there are no URL queries affecting these - loadMeasurements is the relevant function here. This prioritises rendering of the tree, with the downside that a measurements panel flashes in a second later (example). Narrative slide changes (query changes and/or dataset changes) operate the same way.

This PR parses measurements panel URL queries within createStateFromQueryOrJSONs (although the JSON hasn't yet been parsed), and then reads them from redux state in loadMeasurements. Narrative slide changes where the dataset does change are akin to loading a new page. Narrative slide changes where the dataset doesn't change (but query does) apply these changes to the measurements data within createStateFromQueryOrJSONs (via getCollectionDefaultControl), and this is where it feels like the separation isn't as clean as it could be.

We discussed two alternative approaches:

  1. the measurement URL queries are read within loadMeasurements. This is a nicer separation of concerns however narrative slide changes where the dataset doesn't change complicate this, as we never call loadMeasurements. We also want to process all changes within the same action to avoid out-of-sync rendering when a slide changes.
  2. waiting for the measurements JSON to be fetched and loading everything within createStateFromQueryOrJSONs. The downside here is additional complexity in that function. The upsides are rendering all panels simultaneously and making it easier to one day implement measurements queries which modify the tree.
@joverlee521 joverlee521 self-assigned this Oct 23, 2024
@joverlee521
Copy link
Contributor Author

Reminder for myself

If the measurements JSON is fetched upfront and loaded in createStateFromQueryOrJSONs, then auspice.us handling of files will need to updated as well.

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 a pull request may close this issue.

1 participant