Skip to content

Add the Image with Text homepage section - #68

Merged
next-devin merged 7 commits into
mainfrom
issue-30-image-text
Sep 21, 2026
Merged

next-devin merged 7 commits into
mainfrom
issue-30-image-text

Conversation

@next-devin

Copy link
Copy Markdown
Contributor

First slice of #30: the image_text section. FAQ, testimonials and rich text wait on section specs and stay open on the issue.

What changed

  • partials/section_image_text.html: 50/50 image and copy split. Image left or right, ratio square / portrait / landscape / native, eyebrow, heading, richtext body, CTA (primary / secondary / outline), background and text colour. Mobile stacks image first. Toggle off renders nothing; toggle on with no content renders the dashed setup placeholder like the other sections; a missing image with copy present renders a neutral block at the chosen ratio.
  • Settings: new Homepage > Image with Text group and a show_image_text toggle (default off, so existing stores are unchanged). Setting names follow the roster entry verbatim, plus image_text_image_alt. settings_data.json, settings_optional.txt, the localization contract test and all 11 locale files (one placeholder key) are updated.
  • Wiring: included in templates/index.html between Featured Categories and On Sale, so the story block breaks up the two product grids. Easy to move.
  • partials/cta_button.html gains a secondary style mapping to the existing .btn-secondary.
  • Docs: docs/section-specs/image-text.md in the existing spec format; roster, plan, architecture proposal, theme-settings, design-block guide, CLAUDE.md, README.md and CHANGELOG updated for seven shipped sections.
  • assets/main.css recompiled (new ratio and order utilities); css/input.css untouched.

Verification

Reviewer notes

  • CTA model: the roster's three-way image_text_cta_style differs from the older sections' *_cta_style (primary/accent) plus *_cta_outline checkbox. This follows the roster; decide which model new sections standardise on.
  • CTA label fallback reuses homepage.cta.shop_now. A "Learn more" default would need a new key across 11 locales.
  • The section roster JSON in the public skills repo should flip image_text to shipped when this merges.
  • No version bump; recorded under CHANGELOG Unreleased.

🤖 Generated with Claude Code

next-devin and others added 3 commits September 21, 2026 15:21
A 50/50 image and copy split (partials/section_image_text.html) with
image left/right, square/portrait/landscape/native image ratio,
eyebrow, heading, rich-text body, a CTA with primary/secondary/outline
styles, and background and text colours. On mobile it stacks image
first. It is included from templates/index.html between Featured
Categories and On Sale, behind the show_image_text toggle, which is
off by default so existing stores render unchanged. With the toggle on
and nothing configured it renders the same setup placeholder pattern
as the sibling sections, with a localized pointer to the settings
location added to every locale file.

partials/cta_button.html gains a `secondary` style so the section can
reuse the existing .btn-secondary class through the shared helper.
image_text_cta_text follows the localization contract: no schema
default, omitted from settings data, the template applies the
localized fallback.

assets/main.css is rebuilt for the new ratio and ordering utilities
(50298 -> 50536 bytes); css/input.css is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add docs/section-specs/image-text.md in the existing spec format
(render contract, setting map, Figma states, QA checklist, migration
table, gaps), list it in the section-specs index, the roster's Tier 0
table and shipped counts, the Figma plan's section-unit table, the
architecture proposal's migration list, and the settings-to-partials
catalog. Update the homepage section enumerations in CLAUDE.md and
README.md, note the new `secondary` CTA style in the design-block
guide, and add the Unreleased changelog entry.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…gument

settings.* as a filter argument raises a 500 on the platform once the
setting is populated; firstof yields the same string safely.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@next-devin
next-devin marked this pull request as ready for review September 21, 2026 08:30
Comment thread partials/section_image_text.html Outdated
Comment thread partials/section_image_text.html
Comment thread partials/section_image_text.html Outdated
Comment thread partials/section_image_text.html
@kilo-code-bot

kilo-code-bot Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The previous review's heading-color fix at 88f969e remains correct. Since then, three follow-up PRs (#65 sold-out card state, #66 template contract gates, #67 app-hooks contract and membership-key default change) landed on the branch before #68 merged, but their line ranges are not part of the PR #68 diff, so no new inline comments are possible from the PR #68 surface. Cross-checked the PR #68 diff hunks that did change since 88f969e (the CHANGELOG.md image-text entry, the CLAUDE.md settings-row addition, the docs/theme-settings-partials.md section row, the partials/section_image_text.html heading inline-style carry) and found no new defects. The four original findings on partials/section_image_text.html remain resolved by the author's commits 4029e00.

Files Reviewed (PR #68 diff hunks since 88f969e)
  • CHANGELOG.md - 0 issues
  • CLAUDE.md - 0 issues
  • docs/theme-settings-partials.md - 0 issues
  • partials/section_image_text.html - 0 issues
Previous Review Summaries (3 snapshots, latest commit 88f969e)

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

Previous review (commit 88f969e)

Status: No Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Commit 88f969e closes the loop the previous review flagged in the summary-only QA checklist: the heading now carries the configured image_text_text_color as an inline style, so the explicit heading colour from the base stylesheet (and the global heading colour setting) can no longer win over the section-level inline colour. The render contract in docs/section-specs/image-text.md:35 documents why a plain inheritance approach is not enough.

  • partials/section_image_text.html:32 - Inline style="color: ..." is gated on the color setting being truthy, so an unset value never produces an empty style attribute. The conditional correctly mirrors the inheritance path the eyebrow and body already rely on, while working around the explicit heading colour the base stylesheet applies.
  • docs/section-specs/image-text.md:35 - The new sentence matches the implementation: the heading gets the colour via inline style, eyebrow and body inherit at reduced opacity.
Files Reviewed (2 changed files)
  • docs/section-specs/image-text.md - 0 issues
  • partials/section_image_text.html - 0 issues

Previous review (commit 4029e00)

Status: 0 New Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Commit 4029e00 resolves all four findings from the previous review:

  • partials/section_image_text.html:3 - Guard now requires heading or body. Image alone no longer goes live; the setup placeholder renders instead.
  • partials/section_image_text.html:40 - Dead style="primary" kwarg dropped from the outline branch; helper default is now relied on.
  • docs/section-specs/image-text.md:31 - Render contract now documents that |safe trusts the platform's richtext setting type.
  • docs/section-specs/image-text.md:78,126 - Setting map notes and Figma states updated to match the new guard semantics.
  • locales/{de,es,fr}.json - Placeholder caption localized to follow the sibling translation convention; remaining eight locales stay English to match the project-wide placeholder convention.

Follow-up (summary-only, not on a diff line)

  • docs/section-specs/image-text.md:150 - QA checklist still says "the setup placeholder ... disappears once any of image, heading, or body is set." With the new guard, the placeholder disappears only when heading or body is set; an image alone keeps it visible. The checklist is now stale relative to the render contract changed in this PR. Worth a one-line tweak in a follow-up: change "any of image, heading, or body" to "heading or body" so the QA step matches the implementation.
Files Reviewed (5 changed files)
  • docs/section-specs/image-text.md - 0 new issues
  • locales/de.json - 0 new issues
  • locales/es.json - 0 new issues
  • locales/fr.json - 0 new issues
  • partials/section_image_text.html - 0 new issues

Fix these issues in Kilo Cloud

Previous review (commit e0f8d24)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
partials/section_image_text.html 3 Image-only or copy-only configurations render an unbalanced live section with one empty column.
partials/section_image_text.html 60 homepage.placeholder.image_text is an untranslated English settings-help-link string in all 11 locale files.

SUGGESTION

File Line Issue
partials/section_image_text.html 35 `
partials/section_image_text.html 40 Outline branch passes a dead style="primary" argument that the CTA helper ignores.
Files Reviewed (26 files)
  • CHANGELOG.md
  • CLAUDE.md
  • README.md
  • assets/main.css
  • configs/settings_data.json
  • configs/settings_optional.txt
  • configs/settings_schema.json
  • docs/design-block-authoring.md
  • docs/figma-section-library-plan.md
  • docs/section-roster.md
  • docs/section-specs/README.md
  • docs/section-specs/image-text.md
  • docs/sections-architecture-proposal.md
  • docs/theme-settings-partials.md
  • locales/de.json
  • locales/en.default.json
  • locales/es.json
  • locales/fi.json
  • locales/fr.json
  • locales/it.json
  • locales/nl.json
  • locales/no.json
  • locales/pt.json
  • locales/sv.json
  • locales/th.json
  • partials/cta_button.html
  • partials/section_image_text.html
  • templates/index.html
  • tests/test_localization_contracts.py

Fix these issues in Kilo Cloud


Reviewed by minimax-m3 · Input: 0 · Output: 0 · Cached: 0

next-devin and others added 4 commits September 21, 2026 15:45
Review follow-ups on #68: the live guard needs a heading or body, so an
image alone renders the setup placeholder instead of a lone image beside
an empty column; the outline CTA branch drops a redundant style kwarg;
the spec records the |safe trust on the richtext setting; the placeholder
caption is translated in the locales that translate their siblings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The base stylesheet and the global heading colour setting give every
heading an explicit colour, so dropping text-slate-800 left the h2 dark on
a dark custom background. Apply the section colour inline on the heading.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@next-devin
next-devin merged commit 72f9add into main Sep 21, 2026
2 checks passed
@next-devin
next-devin deleted the issue-30-image-text branch September 21, 2026 13:26
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