Create initial story JSON schema #12131
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
swissspidy
added
Type: Documentation
|
Plugin builds for 868d956 are ready 🛎️!
|
miina
reviewed
Aug 22, 2022
timarney
approved these changes
Aug 24, 2022
swissspidy
commented
Aug 30, 2022
swissspidy
commented
Aug 30, 2022
|
@swissspidy On not storing the said values on elements (e.g.
Having a page element type is still relevant as it currently is. The same utils are used for creating a page as for other elements, it has its default element attributes etc. Not sure if storing |
miina
reviewed
Sep 2, 2022
| "type": "object", | ||
| "title": "The default background element", | ||
| "$ref": "#/$defs/shapeElement", | ||
| "$comment": "TODO: Move to story element or remove?" |
There was a problem hiding this comment.
I was wondering whether it makes sense to store the default background element in the story data JSON in the first place, since this could in theory just be a const somewhere in the editor since it's static and never changes.
From what I can see we mainly need this information about the default background element when removing the background element (e.g. an image or video) and need to restore the default.
We can still have the isDefaultBackground property to identify the default in the story JSON, should we ever need a data migration for it.
Does that make sense?
I'm checking now that the |
I didn't have anything in mind, but I can create a ticket to further discuss this idea 👍 The risk with a hardcoded list is that we forget to maintain it, but I think that becomes easier once we have strong TypeScript types for elements. Then one will get yelled at when adding a new unknown property anyway :-) |
|
Size Change: -761 B (0%) Total Size: 2.68 MB
ℹ️ View Unchanged
|
This was referenced Sep 8, 2022
8 tasks
8 tasks
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.
Context
See #12084
Summary
Found some bugs thanks to this:
343f355
Relevant Technical Choices
bin/for nowajv-clifor JSON schema validationajv-formatsto add support for things likeuuidanddate-timetypesTo-do
backgroundOverlayandfontWeightfrom existing stories.Not sure how likely these exists on existing stories though.
showInLibraryfrom ending up in the JSON data. Add a data migration accordingly & remove from schema.type: pageand we register a special "page" element type. Is that still relevant or outdated?current,selection, andstorybasedOnproperty. Seems useless to keep, since this is only used when duplicating elements.User-facing changes
N/A
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
No
Does this PR change what data or activity we track or use?
No
Does this PR have a legal-related impact?
No
Checklist
Type: XYZlabel to the PRFixes #12084