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

Add Minimal schema #2

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Add Minimal schema #2

merged 7 commits into from
Dec 12, 2023

Conversation

mbasaglia
Copy link
Member

Description

This adds the schema for a very basic lottie definition (Subset of the baseline profile).

Once this is merged we can discuss adding schema definitions for more objects in baseline/core profiles.

I've made some of the schema definitions hierarchical so it can map better to an object model (not just JSON validation), this also reduces duplication of certain properties shared by similar objects.

Compatibility Considerations

I've only added objects and properties that are widely supported. Do let me know if there's anything that should be removed or properties that need to be marked as required for a specific object.

@cla-bot cla-bot bot added the cla-signed label Nov 22, 2023
Copy link
Member

@fmalita fmalita left a comment

Choose a reason for hiding this comment

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

I recall your other PR included some scripts to merge everything into a single schema file. Are you planning to add that later?

@mbasaglia
Copy link
Member Author

mbasaglia commented Nov 28, 2023

I recall your other PR included some scripts to merge everything into a single schema file. Are you planning to add that later?

Yea, I wanted to discuss the scripts before adding them to the repo. Maybe we need to figure out profiles as well so the scripts create a full schema for a given profile

@Aidosmf
Copy link
Member

Aidosmf commented Nov 28, 2023

should we add "default" to every property?

@Aidosmf
Copy link
Member

Aidosmf commented Nov 28, 2023

should we move common properties to a different place and reference them where needed, to avoid duplications? For example "w" (width) is used in layers/precomposition-layer.json and /composition/animation.json. Maybe we can create a directory called common or helper for such properties?

@Aidosmf
Copy link
Member

Aidosmf commented Nov 28, 2023

should we add caniuse property for syncing feature names?

schema/composition/animation.json Show resolved Hide resolved
schema/composition/animation.json Show resolved Hide resolved
schema/layers/precomposition-layer.json Show resolved Hide resolved
schema/shapes/group.json Outdated Show resolved Hide resolved
schema/shapes/shape-element.json Show resolved Hide resolved
schema/properties/color-keyframe.json Outdated Show resolved Hide resolved
schema/properties/shape-keyframe.json Outdated Show resolved Hide resolved
schema/properties/shape-keyframe.json Outdated Show resolved Hide resolved
schema/shapes/path.json Show resolved Hide resolved
schema/shapes/rectangle.json Show resolved Hide resolved
@mbasaglia
Copy link
Member Author

I've added a script that merges the schema into a single file. I have neither committed nor ignored the file, we should discuss which approach to have with it. I suggest not having the file committed and having it built with github actions.

@mbasaglia
Copy link
Member Author

mbasaglia commented Nov 29, 2023

should we add "default" to every property?

We would need to have discussions for defaults as separate merge requests

should we add caniuse property for syncing feature names?

I added the proposal #3 I think caniuse should follow the schema, not the other way round.

should we move common properties to a different place and reference them where needed, to avoid duplications? For example "w" (width) is used in layers/precomposition-layer.json and /composition/animation.json. Maybe we can create a directory called common or helper for such properties?

No, I really don't like splitting the schema like that, I think it would make it significantly more difficult to read and maintain. If unrelated objects have similar properties they should be duplicated.

In the schema I'm trying to keep relationships between objects somewhat similar to an object-oriented model.

Copy link
Member

@Aidosmf Aidosmf left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@bodymovin bodymovin left a comment

Choose a reason for hiding this comment

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

lgtm

"title": "Opacity",
"$ref": "#/$defs/properties/value"
},
"sk": {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we specify that skew and skew axis are not supported in layers and that they are only supported as shape transforms?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think some players support them in layers as well, perhaps this can be specified in the profile definitions

@bodymovin bodymovin merged commit 78e1184 into lottie:main Dec 12, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants