Validate style properties value validation (#654)

Shopify · Jun 25, 2024 · a667ee9 · a667ee9
1 parent 664ba3a
commit a667ee9
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 81 deletions.
diff --git a/schemas/theme/setting.json b/schemas/theme/setting.json
@@ -697,6 +697,36 @@
  "additionalProperties": false
  },
 
+ "lengthPattern": {
+ "type": "string",
+ "pattern": "^[0-9]+(px|%)$"
+ },
+
+ "lengthAutoPattern": {
+ "type": "string",
+ "pattern": "^([0-9]+(px|%)|auto|fit-content)$"
+ },
+
+ "lengthNonePattern": {
+ "type": "string",
+ "pattern": "^([0-9]+(px|%)|none|fit-content)$"
+ },
+
+ "negativeLengthPattern": {
+ "type": "string",
+ "pattern": "^-?[0-9]+(px|%)$"
+ },
+
+ "numberPattern": {
+ "type": "string",
+ "pattern": "^[0-9]+$"
+ },
+
+ "negativeNumberPattern": {
+ "type": "string",
+ "pattern": "^-?[0-9]+$"
+ },
+
  "style.layout_panel": {
  "allOf": [{ "$ref": "#/definitions/inputSettingsStandardAttributes" }],
  "properties": {
@@ -1013,16 +1043,17 @@
  "enum": ["nowrap", "wrap", "wrap-reverse"]
  },
  "row-gap": {
- "type": "string",
+ "$ref": "#/definitions/lengthPattern",
  "description": "Defines the size of the gap between the rows of a wrapped flex container."
  },
  "column-gap": {
- "type": "string",
+ "$ref": "#/definitions/lengthPattern",
  "description": "Defines the size of the gap between flex items."
  },
  "gap": {
  "type": "string",
- "description": "Shorthand for row-gap and column-gap. Defines the gaps between flex items."
+ "description": "Shorthand for row-gap and column-gap. Defines the gaps between flex items.",
+ "pattern": "^([0-9]+(px|%) ?){1,2}$"
  },
  "justify-content": {
  "type": "string",
@@ -1077,39 +1108,39 @@
  "type": "object",
  "properties": {
  "flex-grow": {
- "type": "string",
+ "$ref": "#/definitions/numberPattern",
  "description": "Defines the flex grow factor of a flex item, determining how much of the available space in the flex container the item should occupy."
  },
  "flex-shrink": {
- "type": "string",
+ "$ref": "#/definitions/numberPattern",
  "description": "Defines the flex shrink factor of a flex item, determining how much the item should reduce in size compared to the other flex items when space is insufficient."
  },
  "flex-basis": {
- "type": "string",
+ "$ref": "#/definitions/lengthAutoPattern",
  "description": "Defines the initial main size of a flex item, determining the size of the content-box unless otherwise specified by box-sizing."
  },
  "width": {
- "type": "string",
+ "$ref": "#/definitions/lengthAutoPattern",
  "description": "Defines the width of an item."
  },
  "min-width": {
- "type": "string",
+ "$ref": "#/definitions/lengthAutoPattern",
  "description": "Defines the minimum width of an item."
  },
  "max-width": {
- "type": "string",
+ "$ref": "#/definitions/lengthNonePattern",
  "description": "Defines the maximum width of an item."
  },
  "height": {
- "type": "string",
+ "$ref": "#/definitions/lengthAutoPattern",
  "description": "Defines the height of an item."
  },
  "min-height": {
- "type": "string",
+ "$ref": "#/definitions/lengthAutoPattern",
  "description": "Defines the minimum height of an item."
  },
  "max-height": {
- "type": "string",
+ "$ref": "#/definitions/lengthNonePattern",
  "description": "Defines the maximum height of an item."
  }
  }
@@ -1119,92 +1150,97 @@
  "type": "object",
  "properties": {
  "padding": {
- "type": "string",
- "description": "Shorthand that defines the padding on all four sides of an element."
+ "description": "Shorthand that defines the padding on all four sides of an element.",
+ "pattern": "^([0-9]+(px|%) ?){1,4}$"
  },
  "padding-top": {
- "type": "string",
- "description": "Defines the padding on the top side of an element."
+ "description": "Defines the padding on the top side of an element.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-right": {
- "type": "string",
- "description": "Defines the padding on the right side of an element."
+ "description": "Defines the padding on the right side of an element.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-bottom": {
- "type": "string",
- "description": "Defines the padding on the bottom side of an element."
+ "description": "Defines the padding on the bottom side of an element.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-left": {
- "type": "string",
- "description": "Defines the padding on the left side of an element."
+ "description": "Defines the padding on the left side of an element.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-block-start": {
- "type": "string",
- "description": "Defines the logical block start padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical block start padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-block-end": {
- "type": "string",
- "description": "Defines the logical block end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical block end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-block": {
  "type": "string",
- "description": "Shorthand for padding-block-start and padding-block end. Defines the logical block start and end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Shorthand for padding-block-start and padding-block end. Defines the logical block start and end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "pattern": "^([0-9]+(px|%) ?){1,2}$"
  },
  "padding-inline-start": {
- "type": "string",
- "description": "Defines the logical inline start padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical inline start padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-inline-end": {
- "type": "string",
- "description": "Defines the logical inline end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical inline end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/lengthPattern"
  },
  "padding-inline": {
  "type": "string",
- "description": "Shorthand that defines the logical inline start and end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation."
+ "description": "Shorthand that defines the logical inline start and end padding of an element, translating to physical padding based on the element's writing mode, text direction, and text orientation.",
+ "pattern": "^([0-9]+(px|%) ?){1,2}$"
  },
  "margin": {
  "type": "string",
- "description": "Shorthand that defines the margin on all four sides of an element."
+ "description": "Shorthand that defines the margin on all four sides of an element.",
+ "pattern": "^(-?[0-9]+(px|%) ?){1,4}$"
  },
  "margin-top": {
- "type": "string",
- "description": "Defines the margin on the top side of an element."
+ "description": "Defines the margin on the top side of an element.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-right": {
- "type": "string",
- "description": "Defines the margin on the right side of an element."
+ "description": "Defines the margin on the right side of an element.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-bottom": {
- "type": "string",
- "description": "Defines the margin on the bottom side of an element."
+ "description": "Defines the margin on the bottom side of an element.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-left": {
- "type": "string",
- "description": "Defines the margin on the left side of an element."
+ "description": "Defines the margin on the left side of an element.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-block-start": {
- "type": "string",
- "description": "Defines the logical block start margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical block start margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-block-end": {
- "type": "string",
- "description": "Defines the logical block end margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical block end margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-block": {
  "type": "string",
- "description": "Shorthand that defines the logical block start and end margins of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Shorthand that defines the logical block start and end margins of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "pattern": "^(-?[0-9]+(px|%) ?){1,2}$"
  },
  "margin-inline-start": {
- "type": "string",
- "description": "Defines the logical inline start margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical inline start margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-inline-end": {
- "type": "string",
- "description": "Defines the logical inline end margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Defines the logical inline end margin of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "$ref": "#/definitions/negativeLengthPattern"
  },
  "margin-inline": {
  "type": "string",
- "description": "Shorthand that defines both the logical inline start and end margins of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation."
+ "description": "Shorthand that defines both the logical inline start and end margins of an element, translating to physical margin based on the element's writing mode, text direction, and text orientation.",
+ "pattern": "^(-?[0-9]+(px|%) ?){1,2}$"
  }
  }
  },

diff --git a/tests/fixtures/theme-settings-all-settings.json b/tests/fixtures/theme-settings-all-settings.json
@@ -277,14 +277,14 @@
  "default": {
  "flex-direction": "row",
  "column-gap": "20px",
- "row-gap": "2rem",
+ "row-gap": "2px",
  "flex-wrap": "wrap",
  "justify-content": "space-around",
  "align-items": "center",
  "align-content": "space-between",
  "@media (--mobile)": {
  "flex-direction": "column",
- "gap": "0"
+ "gap": "0px"
  }
  }
  },

diff --git a/tests/theme_settings.spec.ts b/tests/theme_settings.spec.ts
@@ -380,13 +380,16 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
  "id": "layout",
  "label": "Layout",
  "default": {
+ "gap": "10px 10px",
+ "row-gap": "10px",
+ "column-gap": "10px",
  "flex-direction": "row",
  "flex-wrap": "wrap",
  "align-items": "center",
  "@media (--mobile)": {
  "flex-direction": "column",
  "align-items": "stretch",
- "gap": "0"
+ "gap": "0px"
  }
  }
  },
@@ -395,11 +398,13 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
  "id": "spacing",
  "label": "Spacing",
  "default": {
- "padding": "10px",
- "margin": "10px",
+ "padding": "10px 20px",
+ "margin": "10px 20px 10px 20px",
  "@media (--mobile)": {
- "margin-top": "0",
- "margin-inline-end": "1rem" 
+ "padding-block": "10px 20px",
+ "margin-top": "0px",
+ "margin-inline": "20px",
+ "margin-inline-end": "1px" 
  }
  }
  },
@@ -410,6 +415,7 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
  "default": {
  "flex-grow": "2",
  "width": "25%",
+ "height": "25px",
  "@media (--mobile)": {
  "width": "100%",
  "flex-grow": "0"
@@ -446,18 +452,14 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
 
  expect(diagnostics).toStrictEqual([
  expect.objectContaining({
- message: expect.stringContaining(
- 'Value is not accepted.',
- )
- })
+ message: expect.stringContaining('Value is not accepted.'),
+ }),
  ]);
- 
+
  expect(diagnostics).toStrictEqual([
  expect.objectContaining({
- message: expect.stringContaining(
- 'style.layout_panel',
- )
- })
+ message: expect.stringContaining('style.layout_panel'),
+ }),
  ]);
  });
 
@@ -509,10 +511,10 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
  }),
  expect.objectContaining({
  message: 'Property flex-shrunk is not allowed.',
- })
+ }),
  ]);
  });
- 
+
  it('should report invalid property values', async () => {
  const settings = `[
  {
@@ -523,6 +525,9 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
  "id": "layout",
  "label": "Layout",
  "default": {
+ "row-gap": "-10px",
+ "column-gap": "-10px",
+ "gap": "-10px -10px",
  "flex-wrap": "rap"
  }
  }
@@ -532,15 +537,19 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
 
  const diagnostics = await validate('config/settings_schema.json', settings);
 
- expect(diagnostics).toStrictEqual([
+ expect(diagnostics).toHaveLength(4);
+ expect(diagnostics).toContainEqual(
  expect.objectContaining({
- message: expect.stringContaining(
- 'Value is not accepted.',
- ),
- })
- ]);
+ message: expect.stringContaining('Value is not accepted.'),
+ }),
+ );
+ expect(diagnostics).toContainEqual(
+ expect.objectContaining({
+ message: expect.stringContaining('String does not match the pattern'),
+ }),
+ );
  });
- 
+
  it('should report invalid breakpoints', async () => {
  const settings = `[
  {
@@ -566,12 +575,9 @@ describe('Module: theme settings validation (config/settings_schema.json)', () =
 
  expect(diagnostics).toStrictEqual([
  expect.objectContaining({
- message: expect.stringContaining(
- 'Property @media (--iphone) is not allowed'
- )
- })
+ message: expect.stringContaining('Property @media (--iphone) is not allowed'),
+ }),
  ]);
  });
-
  });
 });