Skip to content

Show preview sessions and add overridable template blocks - #39

Merged
next-devin merged 3 commits into
mainfrom
theme-preview-indicator
Jul 29, 2026
Merged

Show preview sessions and add overridable template blocks#39
next-devin merged 3 commits into
mainfrom
theme-preview-indicator

Conversation

@next-devin

@next-devin next-devin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

This adds a small preview banner with an Exit preview link. It appears only when the preview cookie is present and does not affect normal visitors.

The PR also:

  • gives platform compatibility, preview, custom CSS, and delight scripts their own named template blocks
  • updates the template check to require those extension points
  • limits hover effects to devices that support hover and keeps mobile behavior explicit

Checks pass: 34 Python tests, the JavaScript suites, theme settings and template checks, Sass compatibility, and CSS drift.

@next-devin

next-devin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Current rollout: Theme Kit #34, Spark #39, and Skills #36. Oscar-owned work is tracked in NextCommerceCo/oscar-prime#5693. The developer-docs draft is closed until these contracts settle.

@next-devin next-devin changed the title Spark: expose preview sessions and enforce overridable global blocks Show preview sessions and add overridable template blocks Jul 29, 2026
@next-devin
next-devin marked this pull request as ready for review July 29, 2026 11:09
Comment thread assets/js/spark-preview.js Outdated

var exitLink = doc.createElement('a');
exitLink.className = 'spark-preview-indicator__exit';
exitLink.href = '/?deactivate-theme=true';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Exiting preview discards the current storefront location

This hard-coded root URL sends a merchant previewing a product, cart, or other deep link back to the homepage. Preserve the current path (and any relevant non-preview query/hash state) while adding the deactivation parameter so the exit action returns them to the same page outside preview.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Comment thread layouts/base.html Outdated
{% endblock platform_compatibility %}

{% block preview_indicator %}
<script type="text/javascript" src="{{ 'js/spark-preview.js'|asset_url }}"></script>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: The preview script is loaded for every storefront visit

The feature self-gates only after this global asset has already been requested, parsed, and executed, so ordinary visitors are not actually untouched and the documented minimal global script set gains a fifth file. Render this script tag only when the preview cookie/session is present, or otherwise load it without adding an unconditional request to every page.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Comment thread scripts/check-templates.py Outdated
r"{%\s*endverbatim(?:\s+.*?)?\s*%}",
re.DOTALL,
)
BLOCK_RE = re.compile(r"{%\s*block\s+([\w-]+)(?:\s|%})")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Required-block detection does not verify balanced block structure

This regex only collects opening block names, so deleting or misspelling an endblock still satisfies the new integrity gate even though layouts/base.html would no longer be a valid template. Since this check is intended to protect extension points, parse matching block/endblock pairs (including named closers) or otherwise validate template syntax rather than checking opening-tag presence alone.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

The incremental diff (commit cb70f67) extends scripts/check-templates.py so that inspect_block_structure runs on every template path instead of only layouts/base.html. Both previously reported findings in scripts/check-templates.py (balanced block/endblock pairs, and per-template coverage) are resolved by this change. The matching test in tests/test_ci_gates.py was updated to assert the new [block-structure] <path> output format and a new test (test_block_structure_is_checked_in_every_template) exercises the per-template path. No new issues were observed on changed lines.

Files Reviewed (2 files in incremental diff)
  • scripts/check-templates.py - 0 new issues (previous block-structure findings resolved)
  • tests/test_ci_gates.py - 0 new issues
Previous Review Summaries (2 snapshots, latest commit d10c1ff)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit d10c1ff)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
scripts/check-templates.py 126 Block-structure validation only runs against layouts/base.html
Files Reviewed (2 files in incremental diff)
  • assets/js/spark-preview.js - 0 new issues (previous finding fixed: exit URL now preserves pathname/search/hash)
  • layouts/base.html - 0 new issues (previous finding fixed: preview script gated on request.COOKIES.preview_theme)
  • scripts/check-templates.py - 1 issue (previous finding fixed: balanced block/endblock pairs now verified)
  • tests/js/spark-preview.test.js - 0 issues
  • tests/test_ci_gates.py - 0 issues

Fix these issues in Kilo Cloud

Previous review (commit e339c60)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
assets/js/spark-preview.js 33 Exit preview discards the current storefront location
layouts/base.html 228 Preview asset loads unconditionally for ordinary visitors
scripts/check-templates.py 26 Required-block gate does not verify balanced block structure
Files Reviewed (7 files)
  • assets/js/spark-platform.js - 0 issues
  • assets/js/spark-preview.js - 1 issue
  • assets/main.css - 0 issues
  • css/input.css - 0 issues
  • layouts/base.html - 1 issue
  • scripts/check-templates.py - 1 issue
  • tests/js/spark-preview.test.js - 0 issues

Fix these issues in Kilo Cloud


Reviewed by minimax-m3 · Input: 26.1K · Output: 2.2K · Cached: 168K

Comment thread scripts/check-templates.py Outdated
else:
try:
base_text = mask_ignored_regions(base_template.read_text(encoding="utf-8"))
base_blocks, block_errors = inspect_block_structure(base_text)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Block-structure validation only runs against layouts/base.html

inspect_block_structure is invoked solely on base_text, so misspelled or unclosed {% block %} / {% endblock %} tags in templates/ and partials/ still pass the integrity gate even though they would break template inheritance just as easily. The previous review called for parsing matching block/endblock pairs to protect extension points; running the same check on every template path would actually deliver that. Either call inspect_block_structure for each template path collected in template_paths(root) (and surface per-file errors), or restrict the gate's wording so it does not claim to validate template syntax beyond base.html.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@next-devin
next-devin merged commit fbfceeb into main Jul 29, 2026
2 checks passed
@next-devin
next-devin deleted the theme-preview-indicator branch July 29, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant