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

Allow block_order on preset #849

Merged
merged 6 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions schemas/manifest_theme.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
{ "uri": "theme/theme_block_entry.json" },
{ "uri": "theme/targetted_block_entry.json" },
{ "uri": "theme/preset_blocks.json" },
{ "uri": "theme/preset.json" },
{ "uri": "theme/local_block_entry.json" }
]
}
66 changes: 66 additions & 0 deletions schemas/theme/preset.json
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"
}
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 encountered an issue where presets with only name and settings could be validated by either of these refs, so by adding blocks as required to the presetWithBlocksHash it ensured there was no such conflict. A preset with only name (and usually settings, otherwise the whole preset's kinda useless) will get validated by the presetWithBlocksArray.

Optionally, we could make blocks required in presetWithBlocksArray too and require a third option for presets with just name and settings. A trivial change, if it's preferred. I'm on the fence, myself.

Copy link
Contributor

@charlespwd charlespwd Nov 26, 2024

Choose a reason for hiding this comment

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

maybe...

"allOf": [
  { "$ref": "#/definitions/presetBase" }
  { "oneOf": [
    { "$ref": "#/definitions/presetBlocksArray" },
    { "$ref": "#/definitions/presetBlocksHash" },
  ]}
]

?

Copy link
Contributor Author

@miazbikowski miazbikowski Nov 26, 2024

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 and settings - though I'd be down to do it if it solved the presets with "only" name and settings more elegantly... Although the one you posted still requires that the data fit into either presetBlocksArray and presetBlocksHash?

Wouldn't it be more like this to accommodate all 3 possible cases?

  { "oneOf": [
    { "$ref": "#/definitions/presetBase" }
    { "$ref": "#/definitions/presetBlocksArray" },
    { "$ref": "#/definitions/presetBlocksHash" },
  ]}

Copy link
Contributor

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

Copy link
Contributor Author

@miazbikowski miazbikowski Nov 26, 2024

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:
image

(overall the presets autocomplete worked with blocks as arrays and blocks as hash)

],
"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
}
}
}
}
33 changes: 20 additions & 13 deletions schemas/theme/preset_blocks.json
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",
Expand All @@ -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."
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
}
},
Expand All @@ -65,9 +75,6 @@
"settings": {
"$ref": "./default_setting_values.json"
},
"blocks": {
"$ref": "#"
},
"static": {
"type": "boolean",
"description": "If the block is rendered statically or not."
Expand Down
16 changes: 1 addition & 15 deletions schemas/theme/section.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,7 @@
"description": "Presets are default configurations of sections that enable users to easily add a section to a JSON template through the theme editor.",
"markdownDescription": "Presets are default configurations of sections that enable users to easily add a section to a [JSON template](https://shopify.dev/docs/themes/architecture/templates/json-templates) through the theme editor.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/sections/section-schema#presets)",
"items": {
"type": "object",
"required": ["name"],
"properties": {
"name": {
"type": "string",
"description": "The preset name, which will show in the Add section picker of the theme editor.",
"markdownDescription": "The preset name, which will show in the Add section 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"
},
"blocks": {
"$ref": "./preset_blocks.json"
}
}
"$ref": "./preset.json"
}
},

Expand Down
16 changes: 1 addition & 15 deletions schemas/theme/theme_block.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,7 @@
"description": "Presets are default configurations of blocks that enable merchants to easily add a block to a JSON template through the theme editor.",
"markdownDescription": "Presets are default configurations of blocks that enable merchants to easily add a block to a JSON template through the theme editor.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/blocks/theme-blocks/schema#presets)",
"items": {
"type": "object",
"required": ["name"],
"properties": {
"name": {
"type": "string",
"description": "The preset name, which displays in the Add block picker of the theme editor.",
"markdownDescription": "The preset name, which displays in the <strong>Add block</strong> picker of the theme editor.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/blocks/theme-blocks/schema#presets)"
},
"settings": {
"$ref": "./default_setting_values.json"
},
"blocks": {
"$ref": "./preset_blocks.json"
}
}
"$ref": "./preset.json"
}
},

Expand Down
9 changes: 7 additions & 2 deletions tests/fixtures/section-schema-preset-blocks-as-hash.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@
"nestedExampleBlock": {
"type": "text"
}
}
},
"block_order": ["nestedExampleBlock"]
},
"exampleBlock3": {
"type": "text"
}
}
},
"block_order": ["exampleBlock", "exampleBlock2", "exampleBlock3"]
}
],
"locales": {
Expand Down
75 changes: 42 additions & 33 deletions tests/fixtures/theme-block-presets-as-hash.json
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"
]
}
]
}