You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
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.
The text was updated successfully, but these errors were encountered:
Follow up to #1848
Redux state for measurements is a bit of a mess right now because of a few of reasons
Copying from @jameshadfield comment
The text was updated successfully, but these errors were encountered: