search

yt-project / widgyts

Widgets for yt
https://widgyts.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
10 stars 10 forks source link

Multiple fields #70

Closed matthewturk closed 3 years ago

matthewturk commented 3 years ago

This is built on #67, and utilizes https://github.com/data-exp-lab/rust-yt-tools/pull/8 .

I'm still working through the details here because I want to make it maximally flexible for tracking the different fields. For instance, it makes a bit of sense to me that the view should track most of the state, but that may not be completely feasible. So I'm attempting to work through this in a reasonable way, but it's still WIP.

matthewturk commented 3 years ago

This is nearing readiness; I should have it ready for review today. However, it's giving me an opportunity to rethink the separation of concerns.

matthewturk commented 3 years ago

This is ready for review, but it relies on the PR in the other repo. If we get that merged, I can release new package on npm, and this will install cleanly.

matthewturk commented 3 years ago

Alright! So I've updated this to require the newest version of @data-exp-lab/yt-tools and it should be good to go.

This pull request does not enable additional fields -- but it does set it up so that it can be done. This should have no user-facing changes to it.

matthewturk commented 3 years ago

This should be reviewable

matthewturk commented 3 years ago

So the issue with this, right now, is that we are dependent on a couple async calls before we can switch the fields. So I need to thread through promises or something to make sure that before we deposit, the field has been added to the container. We can kinda-sorta force this by adding a couple extra requests, or waiting a moment, but I think there's a safer way in there somewhere.

matthewturk commented 3 years ago

Going to merge this as-is.