Skip to content

fix(css): remove max-height constraint on Virtues card imageSection - #303

Merged
jsboige merged 1 commit into
masterfrom
fix/css-virtues-card-balancing
May 21, 2026
Merged

fix(css): remove max-height constraint on Virtues card imageSection#303
jsboige merged 1 commit into
masterfrom
fix/css-virtues-card-balancing

Conversation

@jsboige

@jsboige jsboige commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove max-height: 50% from .imageSection CSS in the Virtues face template (Argumentum_Virtues_Face_fr.json)
  • This constraint artificially capped the image+title area to half the card height, causing visually unbalanced Virtues cards
  • The Fallacies template (Argumentum_Fallacies_Face_fr.json) never had this constraint — both templates now have identical .imageSection CSS

Root Cause

The .imageSection rule included max-height: 50% which forced the image and title (both children of imageSection) to share only 50% of the card's vertical space. The .texte section (description + example) used flex-grow: 1 and took all remaining space, resulting in cards where the text area dominated and the image was compressed.

Test Plan

  • Run pipeline harvest for Virtues cards to generate card images
  • Visually compare a Virtues card before/after (e.g., PK 1.1.1 "Argument réel")
  • Verify image and title now use their natural vertical space
  • Verify text area still renders correctly without overflow
  • Cross-check that Fallacies cards are unaffected (no CSS change)

Before/After CSS Diff

 .imageSection {
     min-height: 0;
     flex-shrink: 1;
-    max-height: 50%;
     display: flex;
     flex-flow: column;
     justify-content: flex-start;

Related to ai-01 analysis of Virtues/Fallacies visual balance discrepancy.

🤖 Generated with Claude Code

The .imageSection in the Virtues face template had max-height: 50%
which artificially capped the image+title area to half the card height,
causing visually unbalanced cards with disproportionate text areas.
The Fallacies template never had this constraint. Removing it lets flex
naturally distribute vertical space between imageSection and texte.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@myia-ai-01 myia-ai-01 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review ai-01 — APPROVE (conditionnel validation visuelle) ✅

Refactor CSS minimal et bien justifié. Convention review checklist :

Diff sanity

  • Scope minimal : +1/-1 sur 1 fichier (Cards/Fallacies/Argumentum_Virtues_Face_fr.json)
  • Aucune autre régression possible : suppression d'1 propriété CSS, pas d'ajout de logique
  • Argument symétrie Fallacies/Virtues : vérifié par grep — template Fallacies (Argumentum_Fallacies_Face_fr.json) n'a pas max-height: 50% sur .imageSection
  • Root cause cohérente : max-height: 50% + titre dans imageSection forçait image+titre à 50% du vertical → texte dominait. Pattern documenté CLAUDE.md "Virtues CSS Balancing"

CI + état

  • ✅ CI 3/3 SUCCESS (build Debug + Release + GitGuardian)
  • ✅ MERGEABLE / CLEAN
  • ✅ 0 conflits

Historique

  • Régression de PR #285 (qui fermait #252). #285 avait introduit max-height: 50% pour limiter overflow texte, mais a créé déséquilibre image. #303 fait machine arrière.

⚠ Points à clarifier (non bloquants)

  1. Convention 1 PR = 1 issue : aucune issue référencée dans le titre/body. Suggéré :

    • Soit lier à #190 (feat(qa): detect Virtues card text overflow + tighten Virtues card copy — OPEN, recouvre le scope)
    • Soit créer une issue dédiée régression CSS post-#285
    • À ajouter en commentaire ou amend si possible
  2. Validation visuelle pré-merge OBLIGATOIRE (test plan body PR explicite, items non cochés) :

    • Harvest Virtues cards (4 langues, pipeline regen)
    • Screenshot before/after carte PK 1.1.1 "Argument réel" + sample 5 cartes (ex: "honnêteté intellectuelle" — issue couleur identifiée #252)
    • Vérifier pas de réintroduction overflow texte (raison initiale du max-height: 50%)
    • Cross-check Fallacies inchangé (régression)
  3. Hypothèse à valider visuellement : sans max-height: 50%, le titre + image peuvent prendre plus de 50% de la carte. Si texte description très long, est-ce qu'il sera tronqué ? Le pattern original CLAUDE.md suggérait flex-basis: 60% + sortie du titre — approche différente, plus protectrice. Le test visuel doit confirmer que retrait simple suffit.

Décision review

APPROVE côté review code/CI

NE PAS MERGER avant que po-2023 ou jsboige confirme :

  • Test visuel before/after (5 cartes minimum)
  • Pas de régression overflow texte description Virtues

Si test visuel positif → merge OK. Sinon → discuter flex-basis: 60% alternative.

🤖 ai-01 cycle 51 review, 2026-05-18T12:30Z

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[NanoClaw] LGTM

CSS fix removes max-height: 100px constraint on Virtues card imageSection — allows images to display at natural size. The diff is large (89KB) because it's a minified JSON card spec with updated CSS, but the actual change is a single CSS property removal. Clean fix.

@jsboige
jsboige merged commit acbc41c into master May 21, 2026
3 checks passed
jsboige added a commit that referenced this pull request May 31, 2026
… (#400)

Long non-breaking titles — notably RU Cyrillic compounds such as
"НЕЙРОЛИНГВИСТИЧЕСКОЕ ПРОГРАММИРОВАНИЕ" — overflowed the .title box and
were clipped mid-word, because the inner `.title > div` uses
`overflow-wrap: normal`. This implements Option B from #303/#353
(auto-shrink JS), chosen over a RU-only CSS class so the fix is
language- and template-agnostic.

frame.js now runs autoFitCardTitles() on each <card> in generateImages(),
right after font preload and before the domtoimage capture. For each
`.title` it binary-searches the largest font-size that fits horizontally
(floor 8px). Titles that already fit are left untouched (no-op), so
non-overflowing Latin titles are unaffected.

Validated via Playwright on the real Fallacies face template + CSS:
- "AD HOMINEM" (Latin):                          84.5px unchanged (no-op)
- "НЕЙРОЛИНГВИСТИЧЕСКОЕ ПРОГРАММИРОВАНИЕ": 84.5 -> 39.1px, now fits
- "ПСЕВДОНАУЧНОСТЬ":                       84.5 -> 51.7px, now fits

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@jsboige
jsboige deleted the fix/css-virtues-card-balancing branch June 1, 2026 21:42
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.

3 participants