Skip to content

Commit e94a6ee

Browse files
six-shopifysix-shopify
authored
Merge pull request #918 from Shopify/fix-conditional-settings-allowed-types
Allow visible_if on more valid settings types
2 parents bfc9084 + 0b5a9ca commit e94a6ee

File tree

2 files changed

+98
-37
lines changed

2 files changed

+98
-37
lines changed

‎schemas/theme/setting.json

+42-14
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,10 @@
629629
"additionalProperties": false
630630
},
631631
"number": {
632-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
632+
"allOf": [
633+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
634+
{ "$ref": "#/definitions/conditionalSetting" }
635+
],
633636
"properties": {
634637
"type": {
635638
"const": "number",
@@ -643,7 +646,8 @@
643646
"default": { "type": "number" },
644647
"label": true,
645648
"info": true,
646-
"id": true
649+
"id": true,
650+
"visible_if": true
647651
},
648652
"additionalProperties": false
649653
},
@@ -701,7 +705,10 @@
701705
},
702706

703707
"radio": {
704-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
708+
"allOf": [
709+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
710+
{ "$ref": "#/definitions/conditionalSetting" }
711+
],
705712
"properties": {
706713
"type": {
707714
"const": "radio",
@@ -715,7 +722,8 @@
715722
"options": { "$ref": "#/definitions/options" },
716723
"label": true,
717724
"info": true,
718-
"id": true
725+
"id": true,
726+
"visible_if": true
719727
},
720728
"required": ["options"],
721729
"additionalProperties": false
@@ -841,7 +849,10 @@
841849
},
842850

843851
"style.layout_panel": {
844-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
852+
"allOf": [
853+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
854+
{ "$ref": "#/definitions/conditionalSetting" }
855+
],
845856
"properties": {
846857
"type": {
847858
"const": "style.layout_panel",
@@ -866,13 +877,17 @@
866877
},
867878
"label": true,
868879
"info": true,
869-
"id": true
880+
"id": true,
881+
"visible_if": true
870882
},
871883
"additionalProperties": false
872884
},
873885

874886
"style.size_panel": {
875-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
887+
"allOf": [
888+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
889+
{ "$ref": "#/definitions/conditionalSetting" }
890+
],
876891
"properties": {
877892
"type": {
878893
"const": "style.size_panel",
@@ -897,13 +912,17 @@
897912
},
898913
"label": true,
899914
"info": true,
900-
"id": true
915+
"id": true,
916+
"visible_if": true
901917
},
902918
"additionalProperties": false
903919
},
904920

905921
"style.spacing_panel": {
906-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
922+
"allOf": [
923+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
924+
{ "$ref": "#/definitions/conditionalSetting" }
925+
],
907926
"properties": {
908927
"type": {
909928
"const": "style.spacing_panel",
@@ -928,13 +947,17 @@
928947
},
929948
"label": true,
930949
"info": true,
931-
"id": true
950+
"id": true,
951+
"visible_if": true
932952
},
933953
"additionalProperties": false
934954
},
935955

936956
"text": {
937-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
957+
"allOf": [
958+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
959+
{ "$ref": "#/definitions/conditionalSetting" }
960+
],
938961
"properties": {
939962
"type": {
940963
"const": "text",
@@ -950,7 +973,8 @@
950973
},
951974
"label": true,
952975
"info": true,
953-
"id": true
976+
"id": true,
977+
"visible_if": true
954978
},
955979
"additionalProperties": false
956980
},
@@ -979,7 +1003,10 @@
9791003
},
9801004

9811005
"textarea": {
982-
"allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
1006+
"allOf": [
1007+
{ "$ref": "#/definitions/inputSettingsStandardAttributes" },
1008+
{ "$ref": "#/definitions/conditionalSetting" }
1009+
],
9831010
"properties": {
9841011
"type": {
9851012
"const": "textarea",
@@ -995,7 +1022,8 @@
9951022
},
9961023
"label": true,
9971024
"info": true,
998-
"id": true
1025+
"id": true,
1026+
"visible_if": true
9991027
},
10001028
"additionalProperties": false
10011029
},

‎tests/section.spec.ts

+56-23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import set from 'lodash.set';
22
import { assert, describe, expect, it } from 'vitest';
3-
import { SETTINGS_TYPES_NOT_SUPPORTING_VISIBLE_IF } from './test-constants';
3+
import { INPUT_SETTING_TYPES, SETTINGS_TYPES_NOT_SUPPORTING_VISIBLE_IF } from './test-constants';
44
import { complete, getService, hover, loadFixture, validateSchema } from './test-helpers';
55

66
const sectionSchema1 = loadFixture('section-schema-1.json');
@@ -29,7 +29,7 @@ describe('JSON Schema validation of Liquid theme section schema tags', () => {
2929
sectionSchema5,
3030
sectionSchema6,
3131
sectionNestedBlocks,
32-
sectionSchemaPresetBlocksAsHash
32+
sectionSchemaPresetBlocksAsHash,
3333
];
3434
for (const sectionSchema of schemas) {
3535
const diagnostics = await validate('sections/section.liquid', sectionSchema);
@@ -211,31 +211,64 @@ describe('JSON Schema validation of Liquid theme section schema tags', () => {
211211
});
212212

213213
it('should validate section schema with conditional settings', async () => {
214-
const sectionSchemaConditionalSettings = loadFixture('section-schema-conditional-settings.json');
214+
const sectionSchemaConditionalSettings = loadFixture(
215+
'section-schema-conditional-settings.json',
216+
);
215217
const diagnostics = await validate('sections/section.liquid', sectionSchemaConditionalSettings);
216218
expect(diagnostics).toStrictEqual([]);
217219
});
218220

219-
it.each(SETTINGS_TYPES_NOT_SUPPORTING_VISIBLE_IF)('should not allow visible_if on %s setting type', async (settingType) => {
220-
const schema = {
221-
settings: [
222-
{
223-
type: settingType,
224-
id: 'test_setting',
225-
label: 'Test Setting',
226-
visible_if: '{{ section.settings.some_setting }}'
227-
}
228-
]
229-
};
230-
231-
const diagnostics = await validate('sections/section.liquid', schema);
232-
expect(diagnostics).toContainEqual(
233-
expect.objectContaining({
234-
message: 'Property visible_if is not allowed.',
235-
severity: 1
236-
})
237-
);
238-
});
221+
const settingsTypesSupportingVisibleIf = INPUT_SETTING_TYPES.filter(
222+
(settingType) =>
223+
!SETTINGS_TYPES_NOT_SUPPORTING_VISIBLE_IF.concat('color_scheme_group').includes(settingType),
224+
);
225+
226+
it.each(settingsTypesSupportingVisibleIf)(
227+
'should allow visible_if on %s setting type',
228+
async (settingType) => {
229+
const schema = {
230+
settings: [
231+
{
232+
type: settingType,
233+
id: 'test_setting',
234+
label: 'Test Setting',
235+
visible_if: '{{ section.settings.some_setting }}',
236+
},
237+
],
238+
};
239+
240+
const diagnostics = await validate('sections/section.liquid', schema);
241+
expect(diagnostics).not.toContainEqual(
242+
expect.objectContaining({
243+
message: 'Property visible_if is not allowed.',
244+
}),
245+
);
246+
},
247+
);
248+
249+
it.each(SETTINGS_TYPES_NOT_SUPPORTING_VISIBLE_IF)(
250+
'should not allow visible_if on %s setting type',
251+
async (settingType) => {
252+
const schema = {
253+
settings: [
254+
{
255+
type: settingType,
256+
id: 'test_setting',
257+
label: 'Test Setting',
258+
visible_if: '{{ section.settings.some_setting }}',
259+
},
260+
],
261+
};
262+
263+
const diagnostics = await validate('sections/section.liquid', schema);
264+
expect(diagnostics).toContainEqual(
265+
expect.objectContaining({
266+
message: 'Property visible_if is not allowed.',
267+
severity: 1,
268+
}),
269+
);
270+
},
271+
);
239272

240273
it('should complete the type property with the generic docs', async () => {
241274
const result = await complete(

0 commit comments

Comments
 (0)