-
Notifications
You must be signed in to change notification settings - Fork 9
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
Allow block_order on preset #849
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
cc6f212
Allow block_order on preset
miazbikowski 3acc901
Allow block_order
miazbikowski d1aa082
Allow nested block_order for hash blocks
miazbikowski 1fedd6d
Add to block presets
miazbikowski 8f7b484
extract name and settings into a reference
miazbikowski 8646b49
Nested 'blocks' must be same type as parent 'blocks'
miazbikowski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
{ | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"title": "Shopify Liquid Section or Block Preset Schema", | ||
"oneOf": [ | ||
{ | ||
"$ref": "#/definitions/presetWithBlocksArray" | ||
}, | ||
{ | ||
"$ref": "#/definitions/presetWithBlocksHash" | ||
} | ||
], | ||
"definitions": { | ||
"presetBase": { | ||
"type": "object", | ||
"required": ["name"], | ||
"properties": { | ||
"name": { | ||
"type": "string", | ||
"description": "The preset name, which will show in the 'Add section' or 'Add block' picker of the theme editor.", | ||
"markdownDescription": "The preset name, which will show in the 'Add section' or 'Add block' picker of the theme editor.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/sections/section-schema#presets)" | ||
}, | ||
"settings": { | ||
"$ref": "./default_setting_values.json" | ||
} | ||
} | ||
}, | ||
"presetWithBlocksArray": { | ||
"type": "object", | ||
"allOf": [ | ||
{ | ||
"$ref": "#/definitions/presetBase" | ||
} | ||
], | ||
"properties": { | ||
"name": true, | ||
"settings": true, | ||
"blocks": { | ||
"$ref": "./preset_blocks.json#/definitions/blocksArray" | ||
} | ||
}, | ||
"additionalProperties": false | ||
}, | ||
"presetWithBlocksHash": { | ||
"type": "object", | ||
"allOf": [ | ||
{ | ||
"$ref": "#/definitions/presetBase" | ||
} | ||
], | ||
"required": ["blocks"], | ||
charlespwd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"properties": { | ||
"name": true, | ||
"settings": true, | ||
"blocks": { | ||
"$ref": "./preset_blocks.json#/definitions/blocksHash" | ||
}, | ||
"block_order": { | ||
"type": "array", | ||
"description": "The order of blocks in the preset.", | ||
"markdownDescription": "The order of blocks in the preset." | ||
}, | ||
"additionalProperties": false | ||
} | ||
} | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,6 @@ | ||
{ | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"title": "Shopify Liquid Preset Blocks Schema", | ||
"oneOf": [ | ||
{ | ||
"$ref": "#/definitions/blocksArray" | ||
}, | ||
{ | ||
"$ref": "#/definitions/blocksHash" | ||
} | ||
], | ||
"definitions": { | ||
"blocksArray": { | ||
"type": "array", | ||
|
@@ -25,7 +17,9 @@ | |
"static": true, | ||
"type": true, | ||
"settings": true, | ||
"blocks": true, | ||
"blocks": { | ||
"$ref": "#/definitions/blocksArray" | ||
}, | ||
"id": { | ||
"type": "string", | ||
"description": "A unique identifier for the block." | ||
|
@@ -50,7 +44,23 @@ | |
"description": "A list of child blocks that you might want to include.", | ||
"markdownDescription": "A list of child blocks that you might want to include.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/sections/section-schema#presets)", | ||
"additionalProperties": { | ||
"$ref": "#/definitions/commonBlockAttributes", | ||
"allOf": [ | ||
{ | ||
"$ref": "#/definitions/commonBlockAttributes" | ||
} | ||
], | ||
"properties": { | ||
"static": true, | ||
"type": true, | ||
"settings": true, | ||
"blocks": { | ||
"$ref": "#/definitions/blocksHash" | ||
}, | ||
Comment on lines
+56
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ezpz! |
||
"block_order": { | ||
"type": "array", | ||
"description": "The order of the blocks in the section." | ||
} | ||
}, | ||
"additionalProperties": false | ||
} | ||
}, | ||
|
@@ -65,9 +75,6 @@ | |
"settings": { | ||
"$ref": "./default_setting_values.json" | ||
}, | ||
"blocks": { | ||
"$ref": "#" | ||
}, | ||
"static": { | ||
"type": "boolean", | ||
"description": "If the block is rendered statically or not." | ||
|
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,48 @@ | ||
{ | ||
"name": "my block", | ||
"blocks": [{ "type": "@app" }, { "type": "@theme" }], | ||
"class": "my-class", | ||
"tag": "custom-element", | ||
"settings": [ | ||
{ | ||
"type": "header", | ||
"content": "header name", | ||
"info": "header info" | ||
"name": "my block", | ||
"blocks": [{ "type": "@app" }, { "type": "@theme" }], | ||
"class": "my-class", | ||
"tag": "custom-element", | ||
"settings": [ | ||
{ | ||
"type": "header", | ||
"content": "header name", | ||
"info": "header info" | ||
}, | ||
{ | ||
"type": "number", | ||
"id": "number", | ||
"label": "my number" | ||
} | ||
], | ||
"presets": [ | ||
{ | ||
"name": "preset name", | ||
"settings": { | ||
"number": 1 | ||
}, | ||
{ | ||
"type": "number", | ||
"id": "number", | ||
"label": "my number" | ||
} | ||
], | ||
"presets": [ | ||
{ | ||
"name": "preset name", | ||
"settings": { | ||
"number": 1 | ||
"blocks": { | ||
"some-block-id": { | ||
"type": "button" | ||
}, | ||
"blocks": { | ||
"some-block-id": { | ||
"type": "button" | ||
"some-other-block-id": { | ||
"type": "image", | ||
"static": true, | ||
"settings": { | ||
"some-setting": "some-value" | ||
}, | ||
"some-other-block-id": { | ||
"type": "image", | ||
"static": true, | ||
"settings": { | ||
"some-setting": "some-value" | ||
"blocks": { | ||
"nested-block-id": { | ||
"type": "text" | ||
} | ||
} | ||
}, | ||
"block_order": ["nested-block-id"] | ||
} | ||
} | ||
] | ||
} | ||
|
||
}, | ||
"block_order": [ | ||
"some-block-id", | ||
"some-other-block-id" | ||
] | ||
} | ||
] | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encountered an issue where presets with only
name
andsettings
could be validated by either of these refs, so by addingblocks
asrequired
to thepresetWithBlocksHash
it ensured there was no such conflict. A preset with onlyname
(and usuallysettings
, otherwise the whole preset's kinda useless) will get validated by thepresetWithBlocksArray
.Optionally, we could make
blocks
required inpresetWithBlocksArray
too and require a third option for presets with justname
andsettings
. A trivial change, if it's preferred. I'm on the fence, myself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I remember when we chatted in person, we agreed it wasn't worth complicating things just to extract out
name
andsettings
- though I'd be down to do it if it solved the presets with "only"name
andsettings
more elegantly... Although the one you posted still requires that the data fit into eitherpresetBlocksArray
andpresetBlocksHash
?Wouldn't it be more like this to accommodate all 3 possible cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that works too so long as presetBase doesn't accept other properties and that the array/hash have blocks & blocks order required. Same same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. The change you proposed ended up being easier to implement - my version was giving me a hard time for a reason I can't figure out, so, if it's same same, then yours works :)
Did some tophatting locally:
(overall the presets autocomplete worked with blocks as arrays and blocks as hash)