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

API proposal: add a way to get a project state pointer #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

felthy
Copy link
Contributor

@felthy felthy commented Mar 1, 2022

We've encountered two use cases so far where we've wanted to react to project state changes:

  1. When building a UI that uses keyframe data, need to be able to react to keyframe changes including when easing curves are changed by the author
  2. To auto-save while authors are using the studio, need to react to all changes anywhere in the state (except metadata like definitionVersion and revisionHistory)

Knowing that val() and onChange() are already exposed from core, all we need is a pointer to the top of the “historic” state. I considered adding a state property to IProject, similar to how ISheetObject exposes props; however the fact that this feature will only be useful in the context of the studio led me to follow the example of createContentOfSaveFile() by adding getProjectState() to IStudio.

@vezwork
Copy link
Contributor

vezwork commented Jun 10, 2022

Hi @felthy thanks for the PR! I really appreciate the inline documentation with usage examples and the explanation of your use cases.

Code-wise this PR looks good to me. I will have to ask @AriaMinaei if this addition makes sense in the context of our plans for the API. @AriaMinaei what do you think? Can we accept this change, or do we want another way to make this possible?

@vezwork
Copy link
Contributor

vezwork commented Jun 10, 2022

quick update: we are discussing this internally and we will keep you updated as we make progress. I can't promise any sort of timeline but I wanted to keep you updated on the status of the PR from our perspective.

@vezwork vezwork added the ⏸ on hold Blocked by internal discussion label Jun 10, 2022
@AriaMinaei
Copy link
Member

@felthy I'm happy to expose this as an experimental API but wanted to check if you guys still need this?

@felthy
Copy link
Contributor Author

felthy commented Jan 27, 2023

Hi @AriaMinaei, yes we are actively using this in our fork for the two use cases in the PR description. Any API that can work for those use cases will be sufficient - this PR is the way I came up with that meets our needs, and also felt in line with the way theatre is already doing things (e.g. val() and onChange() already being exposed publicly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏸ on hold Blocked by internal discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants