Skip to content

Make grid block VTL renderer responsive on small screens#35783

Merged
fmontes merged 3 commits into
mainfrom
claude/responsive-grid-block-7IQYp
May 21, 2026
Merged

Make grid block VTL renderer responsive on small screens#35783
fmontes merged 3 commits into
mainfrom
claude/responsive-grid-block-7IQYp

Conversation

@fmontes
Copy link
Copy Markdown
Member

@fmontes fmontes commented May 20, 2026 •
edited
Loading

Injects a scoped <style> block (once per page via Velocity flag) with a 768px media query that collapses grid columns to full-width stacking. !important overrides are required because column spans are applied as inline styles.

https://claude.ai/code/session_01UwG1xgHsvUanDJjESapt9s

Area.mp4

@claude
Make grid block VTL renderer responsive on small screens
d909f1b 
Injects a scoped <style> block (once per page via Velocity flag) with a
768px media query that collapses grid columns to full-width stacking.
!important overrides are required because column spans are applied as
inline styles.

https://claude.ai/code/session_01UwG1xgHsvUanDJjESapt9s
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

  • This PR fixes #123 or Fixes: #123

  • This PR closes #123 or Closes: #123

  • This PR resolves #123 or Resolves: #123

  • Other supported keywords: fix, fixed, close, closed, resolve, resolved
    Option 2: Link via GitHub UI (Note: won't clear the failed check)

  1. Go to the PR → Development section (right sidebar)

  2. Click "Link issue" and select an existing issue

  3. Push a new commit or re-run the workflow to clear the failed check
    Option 3: Use branch naming
    Create a new branch with one of these patterns:

  • 123-feature-description (number at start)

  • issue-123-feature-description (issue-number at start)

  • feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label May 20, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026 •
edited
Loading

Claude finished @fmontes's task in 1m 17s —— View job

Review

The refactor is solid: dropping !important by moving span into a --grid-span custom property and pinning the media-query override to a class with equal specificity is a clean win over the inline grid-column: span N it replaced. Nothing in the diff looks broken.

A few things worth a second look:

1. PR body is stale. It still says "!important overrides are required because column spans are applied as inline styles." That hasn't been true since commit 3ac67a7c — the inline style now only sets the custom property, so grid-column is fully class-driven. Worth updating the description so a future reader doesn't get confused.

2. <style> injected mid-<body>. Per HTML5 spec, <style> in body without the (deprecated/unimplemented) scoped attribute is technically a parse error. Every modern browser accepts it, so this is pragmatic, not blocking — but it does mean the styles also won't load if a downstream sanitizer/CMS filter strips body <style> tags. Worth knowing.

3. $__gridBlockStyleEmitted lifetime. In Velocity, #set writes to the current context. Inside a single page render that's exactly what you want (one emission per page). But this template is also parsed by render.vtl recursively for nested content blocks, and Velocity contexts can be shared with parent contexts in some configurations. If a parent renderer mutates this var or if the context is pooled/reused between requests in some path, the style block could be silently skipped on a later render with no visible grid. Probably fine in practice, but worth a quick check — and the variable name being a magic string with no namespace ($__gridBlockStyleEmitted) makes accidental collisions cheap.

4. Generic class names. .grid-block and .grid-block__column are plausible class names a customer site might already use. The @media rule will collapse anything matching .grid-block to a single column under 768px. If a tenant has unrelated .grid-block markup, they get unexpected stacking. Consider scoping to div[data-type="gridBlock"] / div[data-type="gridColumn"] to match the data-attribute contract that commit 8b64772a just restored. That also makes the editor's hook and the styling hook the same selector.

5. Hard-coded 768px breakpoint. Not a bug, but customers that want a different mobile breakpoint can't override this without !important of their own (since the media query rule is in a page-injected <style> block at the same specificity as anything they'd write in a stylesheet, but loaded after their stylesheet — order varies). Consider whether this breakpoint belongs in a CSS custom property or in a site-level theme hook.

6. Pre-existing, not introduced here: if item.content has more entries than $item.attrs.columns (which is validated to size 2), gridCols.get(colIndex) for colIndex >= 2 will throw IndexOutOfBoundsException. Worth a follow-up — but not this PR's problem.

None of the above block the PR. (3) and (4) are the ones I'd want eyes on before merging.

  • Read changes
  • Analyze responsive CSS / specificity
  • Velocity scoping flag review
  • XSS / escape review (span value is regex-validated to [0-9]+ and range-checked, safe to interpolate)
  • Posted review
    • Branch: claude/responsive-grid-block-7IQYp

@fmontes fmontes linked an issue May 21, 2026 that may be closed by this pull request
Make grid block VTL renderer responsive on small screens #35791
Open
5 tasks
@fmontes fmontes added this pull request to the merge queue May 21, 2026
@fmontes fmontes removed this pull request from the merge queue due to a manual request May 21, 2026
fmontes and others added 2 commits May 21, 2026 10:47
@fmontes @claude
Refactor grid block responsive styles to drop !important
3ac67a7 
Replaces inline grid-column spans with a --grid-span CSS custom property
applied via the .grid-block__column class, so the 768px media query can
override grid-column directly without !important. Moves the scoped
<style> inside the .grid-block <div> to guarantee a valid parent even
when a grid block is nested in <blockquote>/<table>/etc. Drops the
data-type attribute selectors in favor of the existing class selectors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fmontes @claude
Restore data-type attributes on grid block markup
8b64772 
Keeps data-type="gridBlock" and data-type="gridColumn" on the rendered
elements so downstream consumers can still target them. CSS selectors
still use the .grid-block / .grid-block__column classes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fmontes fmontes enabled auto-merge May 21, 2026 16:51
@fmontes fmontes added this pull request to the merge queue May 21, 2026
Merged via the queue into main with commit b47fc03 May 21, 2026
51 checks passed
@fmontes fmontes deleted the claude/responsive-grid-block-7IQYp branch May 21, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oidacra oidacra oidacra approved these changes

@hmoreras hmoreras hmoreras approved these changes

@dario-daza dario-daza dario-daza approved these changes

Assignees

No one assigned

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

dotCMS - Product Planning
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make grid block VTL renderer responsive on small screens

5 participants

@fmontes @oidacra @hmoreras @dario-daza @claude