narratives load sidecar files from cache; #1312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eharkins
changed the title
|
Does this still need merging / review? |
eharkins
force-pushed
the
bug-narratives-frequencies
branch
from
3c9d28c to
0db3a38
Compare
eharkins
force-pushed
the
bug-narratives-frequencies-cache
branch
from
fdec8ea to
821a116
Compare
As James and I sketched out verbally over the phone, this tries to set up then functions for promises in the dataset cache which are initiated at the start of loading a narrative with sidecar files. This has been minimally tested with the frequencies file and seems to work on the surface.
eharkins
force-pushed
the
bug-narratives-frequencies-cache
branch
from
821a116 to
daea0d4
Compare
|
Thanks Trevor for reminding me about this. It needs to probably just get merged into #1305 since it is on top of that larger PR which lacks this small but important piece. Then further review and testing is definitely needed on #1305 before we merge, which I am working on now but would also appreciate your feedback. |
jameshadfield
added a commit
that referenced
this pull request
Aug 11, 2021
These changes were motivated by [#1283](#1283) which arose as we used different code paths for loading a dataset viz and a narrative. Here we represent each dataset by a `Dataset` object. This is used for stand alone datasets, each dataset in a tangletree, and each dataset in a narrative. Each dataset instance describes the various API endpoints of datafiles for each dataset, manages fetching of these datafiles and, where appropriate, can dispatch data to update redux state. This has been tested on various single datasets, tangle-trees, and narratives in this repo. Notably, this commit breaks narratives with multiple datasets; this will be fixed in a subsequent commit to reflect Eli's work in PR #1312.
jameshadfield
pushed a commit
that referenced
this pull request
Aug 11, 2021
This complements the previous commit to allow narrative slide-changes to change datasets by querying the cache set up at narrative load. Appropriate sidecar files are also loaded, and we ensure that sidecar data from the previous dataset is not displayed. This work is based upon PR #1312 by eharkins. Co-authored-by: eharkins <[email protected]>
jameshadfield
pushed a commit
that referenced
this pull request
Aug 11, 2021
This complements the previous commit to allow narrative slide-changes to change datasets by querying the cache set up at narrative load. Appropriate sidecar files are also loaded, and we ensure that sidecar data from the previous dataset is not displayed. This work is based upon PR #1312 by eharkins. Co-authored-by: eharkins <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As James and I sketched out verbally
over the phone, this tries to set up
then functions for promises in the dataset
cache which are initiated at the start
of loading a narrative with sidecar files.
This has been minimally tested with
the frequencies file and seems to work
on the surface.
@jameshadfield here's what we discussed, needs more testing.