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

All prop sequencing #48

Merged
merged 14 commits into from
Nov 2, 2021
Merged

Conversation

cory-glooh
Copy link
Contributor

Allows sequencing of all prop types, with the ability to override the interpolator. (including compound types, but not supported by the studio yet)

For a value to be sequencable, it needs to either be a number, or implement the functions: Sanitize & Interpolate. which by default, String and Compound types don't do, as I can't interpret the general usage for those types. so I've left it up to the user.

Colours are done using TinyColor2 thanks to Tom for reminding me not only hex colours exist!

@cory-glooh cory-glooh mentioned this pull request Oct 24, 2021
Copy link
Member

@AriaMinaei AriaMinaei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this PR! Adds a bunch of features in one go! I added a few comments which are all minor changes.

theatre/core/src/propTypes/index.ts Outdated Show resolved Hide resolved
theatre/core/src/propTypes/index.ts Outdated Show resolved Hide resolved
packages/playground/src/shared/dom/Scene.tsx Outdated Show resolved Hide resolved
theatre/core/src/propTypes/index.ts Outdated Show resolved Hide resolved
theatre/core/src/sheetObjects/SheetObject.ts Outdated Show resolved Hide resolved
if (typeof defaultValue !== 'number') {

if (
typeof defaultValue !== 'number' &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do we need to explicitly check for a number type here now? Because I think we offloaded that responsibility to the prop types, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do, as no prop config on the tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a bug with the tests then, so we should fix this where the tests are defined

theatre/globals.d.ts Show resolved Hide resolved
theatre/studio/src/StudioStore/StudioStore.ts Outdated Show resolved Hide resolved
@AriaMinaei AriaMinaei merged commit 4a65c6e into theatre-js:main Nov 2, 2021
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 this pull request may close these issues.

None yet

2 participants