Add setting option icons and number bounds to the setting schema - #1831
Conversation
|
|
||
| describe('Module: theme settings validation (config/settings_schema.json)', () => { | ||
| describe('Unit: option icons and number bounds', () => { | ||
| it('select setting allows an option icon', async () => { |
There was a problem hiding this comment.
Is there a way we'd need to gate these changes to behind the flag? Or are we good just allowing it all to be discoverable in theme check? Core will reject it.
There was a problem hiding this comment.
can't really add flags to this one, as they're just static files that theme check and the vs code extension pull from main. i did call this limitation out on slack. i think given the backend is still the source of truth, it's okay if these fields are rejected for now until it fully rolls out. worst case is someone does this manually (rare), or their agents read this file and start using them (higher chance but also rare)
i think the consensus for this repo is it's okay to merge early for us to test internally, so i'll do that. we can rip it out if we need to
| "description": "The smallest value the setting is expected to take. Supports at most one decimal digit. The setting still renders as a plain number field, so the input isn't clamped, but the default and any option values must fall within the bounds.", | ||
| "markdownDescription": "The smallest value the setting is expected to take. Supports at most one decimal digit.\n\nThe setting still renders as a plain number field, so the input isn't clamped, but `default` and `options[].value` must fall within the bounds.\n\n---\n\n[Shopify reference](https://shopify.dev/docs/themes/architecture/settings/input-settings#number)" | ||
| }, | ||
| "max": { |
There was a problem hiding this comment.
Core also validates for number:
- max >= min
- default is within min / max
- options[].value is within min / max
Do we need to make sure we cover that here?
There was a problem hiding this comment.
we can't quite cover those here. the JSON schema can't compare fields against each other, so we can't really express max >= min or "default must be within bounds"
the backend will stay the source of truth for this. the tests make sure no one assumes that this is covered in the JSON schema. the only way around this would be to create a custom theme check in Shopify/theme-tools
i'll merge this as-is, and we can discuss next week if we should add a check
karreiro
left a comment
karreiro
left a comment
There was a problem hiding this comment.
Thank you for this PR, @lukeh-shopify!
I've sent you a video on Slack about grid being a valid value for the icon key -- please, let me know your thoughts about that.
Thanks again for this PR!
There was a problem hiding this comment.
Thanks again for this PR, @lukeh-shopify!
Once we address @rjur11's comments, I'm fine to merge (thank you, Rana, for clarifying the grid details on Slack!).
6d7b8a1 to
8c52498
Compare
Adds the new setting kinds from issues-merchant-workflows issue 2669: - `settingIcon`: an enum of the 64 supported snake_case stable icon IDs, with an error message that points merchants and theme developers away from Polaris component names (`LayoutColumns3Icon`) and kebab-case admin handles (`layout-columns-3`). - `selectOptions`: an optional `icon` on each option. - `radioOptions`: a new definition for `radio` settings that rejects `icon` with a message pointing at `select`, so radio settings no longer silently accept an unsupported attribute. The shared `options` definition is unchanged. - `numberOptions`: a new definition for suggested `value`/`label`/`icon` entries on `number` settings, where `value` is a required number with at most one decimal digit. - `number`: new `min`, `max`, `icon` and `options` attributes, and one decimal digit of precision on `default`, `min` and `max`. The icon list and the precision rules mirror Core's `Theme::SettingOptionIcon` and spec validator, so the editor, the platform and theme-check agree on what is valid. theme-check downloads these schemas from this repository and uses them for its ValidSchema check. Assisted-By: devx/0e7ab314-3226-4582-ac8d-b65bca9cbfd8
8c52498 to
ce74c55
Compare
3 participants
What are you trying to accomplish?
Add schema support and validation for new optional setting properties:
selectsettings: options can now specify aniconnumbersettings: new optionalmin,max,icon, andoptionsmetadataAllows theme check to pass for these before submitting them to the server
What approach did you use?
settingIcondefinition, an enum of the supportedsnake_caseicon IDsnumberOptionsdefinitionmin/maxto thenumberdefinition and option values to enforce at most one decimal digitradioOptionsdefinition so radio options keep their existing shape but rejecticonwith a message pointing to select settings