-
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
Conditional theme settings and split theme-settings unit tests into separate suites #899
Merged
+1,341
−761
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
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
Large diffs are not rendered by default.
Oops, something went wrong.
140 changes: 140 additions & 0 deletions
140
tests/fixtures/section-schema-conditional-settings.json
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,140 @@ | ||
{ | ||
"name": "Custom Section", | ||
"settings": [ | ||
{ | ||
"type": "header", | ||
"content": "Advanced Settings", | ||
"visible_if": "{{ section.settings.show_advanced }}", | ||
"info": "Configure advanced options below" | ||
}, | ||
{ | ||
"type": "checkbox", | ||
"id": "enable_features", | ||
"label": "Enable features", | ||
"default": true, | ||
"visible_if": "{{ section.settings.show_advanced }}" | ||
}, | ||
{ | ||
"type": "color_background", | ||
"id": "bg_color", | ||
"label": "Background Color", | ||
"default": "#ffffff", | ||
"visible_if": "{{ section.settings.enable_custom_colors }}" | ||
}, | ||
{ | ||
"type": "color_scheme", | ||
"id": "color_scheme", | ||
"label": "Color scheme", | ||
"visible_if": "{{ section.settings.enable_custom_colors }}" | ||
}, | ||
{ | ||
"type": "color", | ||
"id": "text_color", | ||
"label": "Text Color", | ||
"default": "#000000", | ||
"visible_if": "{{ section.settings.enable_custom_colors }}" | ||
}, | ||
{ | ||
"type": "font_picker", | ||
"id": "heading_font", | ||
"label": "Heading Font", | ||
"default": "helvetica_n4", | ||
"visible_if": "{{ section.settings.enable_custom_typography }}" | ||
}, | ||
{ | ||
"type": "html", | ||
"id": "custom_html", | ||
"label": "Custom HTML", | ||
"default": "<p>Add your HTML here</p>", | ||
"visible_if": "{{ section.settings.enable_custom_code }}" | ||
}, | ||
{ | ||
"type": "image_picker", | ||
"id": "background_image", | ||
"label": "Background image", | ||
"visible_if": "{{ section.settings.enable_background }}" | ||
}, | ||
{ | ||
"type": "inline_richtext", | ||
"id": "caption", | ||
"label": "Caption", | ||
"default": "Add your caption", | ||
"visible_if": "{{ section.settings.show_caption }}" | ||
}, | ||
{ | ||
"type": "link_list", | ||
"id": "menu", | ||
"label": "Menu", | ||
"default": "main-menu", | ||
"visible_if": "{{ section.settings.show_navigation }}" | ||
}, | ||
{ | ||
"type": "liquid", | ||
"id": "custom_liquid", | ||
"label": "Custom Liquid", | ||
"default": "{% if customer %}Hello {{ customer.name }}{% endif %}", | ||
"visible_if": "{{ section.settings.enable_custom_code }}" | ||
}, | ||
{ | ||
"type": "range", | ||
"id": "content_width", | ||
"label": "Content width", | ||
"min": 400, | ||
"max": 1600, | ||
"step": 100, | ||
"unit": "px", | ||
"default": 1200, | ||
"visible_if": "{{ section.settings.width_type == 'custom' }}" | ||
}, | ||
{ | ||
"type": "richtext", | ||
"id": "content", | ||
"label": "Content", | ||
"default": "<p>Add your content here</p>", | ||
"visible_if": "{{ section.settings.show_content }}" | ||
}, | ||
{ | ||
"type": "select", | ||
"id": "layout_type", | ||
"label": "Layout type", | ||
"options": [ | ||
{ | ||
"value": "grid", | ||
"label": "Grid" | ||
}, | ||
{ | ||
"value": "slider", | ||
"label": "Slider" | ||
} | ||
], | ||
"default": "grid", | ||
"visible_if": "{{ section.settings.enable_layout_options }}" | ||
}, | ||
{ | ||
"type": "text_alignment", | ||
"id": "text_align", | ||
"label": "Text alignment", | ||
"default": "center", | ||
"visible_if": "{{ section.settings.enable_custom_layout }}" | ||
}, | ||
{ | ||
"type": "url", | ||
"id": "button_link", | ||
"label": "Button link", | ||
"visible_if": "{{ section.settings.show_button }}" | ||
}, | ||
{ | ||
"type": "video_url", | ||
"id": "video_url", | ||
"label": "Video URL", | ||
"accept": ["youtube", "vimeo"], | ||
"visible_if": "{{ section.settings.media_type == 'video' }}" | ||
}, | ||
{ | ||
"type": "video", | ||
"id": "video_file", | ||
"label": "Video file", | ||
"visible_if": "{{ section.settings.media_type == 'video' }}" | ||
} | ||
] | ||
} |
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,202 @@ | ||
{ | ||
"name": "Content Block", | ||
"settings": [ | ||
{ | ||
"type": "select", | ||
"id": "layout_style", | ||
"label": "Layout style", | ||
"options": [ | ||
{ | ||
"value": "standard", | ||
"label": "Standard" | ||
}, | ||
{ | ||
"value": "custom", | ||
"label": "Custom" | ||
} | ||
], | ||
"default": "standard" | ||
}, | ||
{ | ||
"type": "range", | ||
"id": "width_percent", | ||
"label": "Width", | ||
"min": 25, | ||
"max": 100, | ||
"step": 5, | ||
"unit": "%", | ||
"default": 100, | ||
"visible_if": "{{ block.settings.layout_style == 'custom' }}" | ||
}, | ||
{ | ||
"type": "select", | ||
"id": "border_style", | ||
"label": "Border style", | ||
"options": [ | ||
{ | ||
"value": "none", | ||
"label": "None" | ||
}, | ||
{ | ||
"value": "solid", | ||
"label": "Solid" | ||
}, | ||
{ | ||
"value": "dashed", | ||
"label": "Dashed" | ||
} | ||
], | ||
"default": "none" | ||
}, | ||
{ | ||
"type": "range", | ||
"id": "border_width", | ||
"label": "Border width", | ||
"min": 1, | ||
"max": 10, | ||
"step": 1, | ||
"unit": "px", | ||
"default": 1, | ||
"visible_if": "{{ block.settings.border_style != 'none' }}" | ||
}, | ||
{ | ||
"type": "range", | ||
"id": "border_opacity", | ||
"label": "Border opacity", | ||
"min": 0, | ||
"max": 100, | ||
"step": 5, | ||
"unit": "%", | ||
"default": 100, | ||
"visible_if": "{{ block.settings.border_style != 'none' }}" | ||
}, | ||
{ | ||
"type": "checkbox", | ||
"id": "enable_background", | ||
"label": "Enable background", | ||
"default": false | ||
}, | ||
{ | ||
"type": "select", | ||
"id": "background_type", | ||
"label": "Background type", | ||
"options": [ | ||
{ | ||
"value": "color", | ||
"label": "Color" | ||
}, | ||
{ | ||
"value": "image", | ||
"label": "Image" | ||
}, | ||
{ | ||
"value": "video", | ||
"label": "Video" | ||
} | ||
], | ||
"default": "color", | ||
"visible_if": "{{ block.settings.enable_background }}" | ||
}, | ||
{ | ||
"type": "image_picker", | ||
"id": "background_image", | ||
"label": "Background image", | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'image' }}" | ||
}, | ||
{ | ||
"type": "url", | ||
"id": "video_url", | ||
"label": "Video URL", | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'video' }}" | ||
}, | ||
{ | ||
"type": "checkbox", | ||
"id": "video_loop", | ||
"label": "Loop video", | ||
"default": true, | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'video' }}" | ||
}, | ||
{ | ||
"type": "color_background", | ||
"id": "bg_color", | ||
"label": "Background Color", | ||
"default": "#ffffff", | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'color' }}" | ||
}, | ||
{ | ||
"type": "color_scheme", | ||
"id": "color_scheme", | ||
"label": "Color scheme", | ||
"visible_if": "{{ block.settings.enable_custom_colors }}" | ||
}, | ||
{ | ||
"type": "color", | ||
"id": "text_color", | ||
"label": "Text Color", | ||
"default": "#000000", | ||
"visible_if": "{{ block.settings.enable_custom_colors }}" | ||
}, | ||
{ | ||
"type": "font_picker", | ||
"id": "heading_font", | ||
"label": "Heading Font", | ||
"default": "helvetica_n4", | ||
"visible_if": "{{ block.settings.enable_custom_typography }}" | ||
}, | ||
{ | ||
"type": "html", | ||
"id": "custom_html", | ||
"label": "Custom HTML", | ||
"default": "<p>Add your HTML here</p>", | ||
"visible_if": "{{ block.settings.enable_custom_code }}" | ||
}, | ||
{ | ||
"type": "inline_richtext", | ||
"id": "caption", | ||
"label": "Caption", | ||
"default": "Add your caption", | ||
"visible_if": "{{ block.settings.show_caption }}" | ||
}, | ||
{ | ||
"type": "link_list", | ||
"id": "menu", | ||
"label": "Menu", | ||
"default": "main-menu", | ||
"visible_if": "{{ block.settings.show_navigation }}" | ||
}, | ||
{ | ||
"type": "liquid", | ||
"id": "custom_liquid", | ||
"label": "Custom Liquid", | ||
"default": "{% if customer %}Hello {{ customer.name }}{% endif %}", | ||
"visible_if": "{{ block.settings.enable_custom_code }}" | ||
}, | ||
{ | ||
"type": "richtext", | ||
"id": "content", | ||
"label": "Content", | ||
"default": "<p>Add your content here</p>", | ||
"visible_if": "{{ block.settings.show_content }}" | ||
}, | ||
{ | ||
"type": "text_alignment", | ||
"id": "text_align", | ||
"label": "Text alignment", | ||
"default": "center", | ||
"visible_if": "{{ block.settings.enable_custom_layout }}" | ||
}, | ||
{ | ||
"type": "video_url", | ||
"id": "video_url_with_hosts", | ||
"label": "Video URL", | ||
"accept": ["youtube", "vimeo"], | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'video' }}" | ||
}, | ||
{ | ||
"type": "video", | ||
"id": "video_file", | ||
"label": "Video file", | ||
"visible_if": "{{ block.settings.enable_background and block.settings.background_type == 'video' }}" | ||
} | ||
] | ||
} |
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
Oops, something went wrong.
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.
Not sure how feasible this is so it's just a question — but rather than
for each forbidden setting type, test that the diagnostic correctly forbids it
maybe it should be
gather the diagnostic result for every setting type, and check that the result list contains exactly the list of forbidden types
I feel like that's maybe testing a more useful property. (I know you already have the handwritten schemas with allowed conditional settings, but if there's a list of all settings types at hand somewhere this could bolster the handwritten stuff with a broader property-based test.)
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.
This is an interesting way to look at it! I had considered an approach similar to what you have in mind, but the issue came down to consciously handwriting invalid settings in with our valid ones. My concern was that it would make that fixture a bit too difficult to scan and compromises its value as a self documenting example of valid settings.
The approach I opted for instead is to include all valid setting types that support conditional settings within the
theme-block-conditional-settings.json
fixture and then only targeting the ones we explicitly do not wish to support (as listed in Clauderic's decision figjam).