Skip to content

fix(configurator): stop full-viewport sizing from clipping inside WP admin - #463

Merged
jackgranatowski merged 4 commits into
mainfrom
claude/configurator-sync-pr-460-461-g1f2w4
Jul 1, 2026
Merged

fix(configurator): stop full-viewport sizing from clipping inside WP admin#463
jackgranatowski merged 4 commits into
mainfrom
claude/configurator-sync-pr-460-461-g1f2w4

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The configurator's root shell used w-screen h-screen (viewport units), which is correct for the standalone hosted app that owns the whole page.
  • Embedded in WP admin, the app mounts inside the host's normal document flow instead of the document body. The viewport-sized root then overflowed past the right edge of the WP admin content area, while the host's own layout chrome (.wrap margin, #wpcontent padding) left a visible gap above and to the left of it.
  • Switch to w-full h-full when running embedded, reusing the existing isEmbedded() boundary already used for persistence (window.slashedApp presence), and keep w-screen h-screen for the standalone case.
  • This is the framework half of a paired fix; the WordPress plugin half (codeslash-dev/SLASHED-Plugins) re-syncs this change and adds plugin-side CSS to give the app's WP-admin ancestor chain a real height so h-full has something to fill.

Test plan

  • npx svelte-check — 0 errors
  • npm run build (configurator) — builds cleanly
  • npm run check:version — in sync (no version-related files touched)

🤖 Generated with Claude Code

https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved app sizing when running inside an embedded host, so the interface fills the available container instead of forcing viewport-sized dimensions.
    • Preserved the existing full-screen layout for standalone use.
  • New Features
    • Added host-environment detection helper to reliably distinguish embedded hosting from REST persistence configuration.
  • UI/UX Improvements
    • Updated color status rendering to use consistent derived variants and improved swatch tooltips and help text clarity.

…admin

w-screen/h-screen are correct for the standalone hosted configurator,
which owns the whole page. Embedded in WP admin, the app mounts inside
the host's normal document flow, so the viewport-sized root overflowed
past the right edge of the content area while the host's own layout
chrome left a visible gap above and to the left of it. Switch to
w-full/h-full when running embedded, matching the existing
isEmbedded() boundary already used for persistence.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4576fbc0-674b-43ff-af7d-6ba8f2d763ce

📥 Commits

Reviewing files that changed from the base of the PR and between dbdf898 and 5aae950.

📒 Files selected for processing (1)
  • configurator/src/components/panels/ColorsPanel.svelte
📝 Walkthrough

Walkthrough

App.svelte now detects host embedding through a persistence helper and switches its root sizing classes accordingly. ColorsPanel adds shared swatch hover labels, updates brand/curve/mini palette text and markup, and replaces the status palette strip with derived status variants.

Changes

Embedded sizing behavior

Layer / File(s) Summary
Detect host container
configurator/src/persistence.ts, configurator/src/App.svelte
Adds hasWpBoot(), imports it into App.svelte, and derives an embedded flag from the host boot object.
Apply root sizing
configurator/src/App.svelte
Switches the root container between w-full h-full and w-screen h-screen based on the embedded flag.

Colors panel rendering

Layer / File(s) Summary
Shared swatch labels and copy
configurator/src/components/panels/ColorsPanel.svelte
Adds a reusable swatchTip snippet, updates brand source help text, and applies hover labels to brand, curve, and mini-palette swatches.
Status variant grid
configurator/src/components/panels/ColorsPanel.svelte
Replaces the numeric status palette strip with a derived-variant grid built from STATUS_VARIANTS and paintTheme() for light and dark swatches.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • codeslash-dev/SLASHED#429: Both PRs touch configurator/src/components/panels/ColorsPanel.svelte, specifically updating how color swatches are rendered and annotated.
  • codeslash-dev/SLASHED#443: Both PRs touch the WordPress embedding seam in lib/persistence.ts and App.svelte.
  • codeslash-dev/SLASHED#461: Both PRs change status palette rendering in configurator/src/components/panels/ColorsPanel.svelte.

Suggested labels

codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main fix: preventing the configurator from using full-viewport sizing inside WordPress admin.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/configurator-sync-pr-460-461-g1f2w4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix configurator root sizing when embedded in WP admin

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Detect embedded mode via existing isEmbedded() boundary.
• Use w-full h-full when embedded to avoid WP admin overflow/gaps.
• Keep w-screen h-screen for standalone hosted configurator viewport sizing.
Diagram

graph TD
  host{{"Host page"}} --> app["Configurator App"] --> persist["persistence.ts"] --> dec{"isEmbedded()?"}
  dec -->|"yes"| emb["Root: w-full/h-full"]
  dec -->|"no"| std["Root: w-screen/h-screen"]

  subgraph Legend
    direction LR
    _ext{{"External host"}} ~~~ _cmp["Component/module"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Always use `w-full/h-full` and enforce full-height in standalone via global CSS
  • ➕ Eliminates runtime branching in the root component
  • ➕ Keeps sizing behavior purely CSS-driven
  • ➖ Requires ensuring html, body, #app (or equivalent) have explicit height in every standalone host
  • ➖ More likely to regress if the standalone hosting shell changes
2. Use positioning (`fixed`/`absolute`) to fill available area instead of width/height utilities
  • ➕ Can avoid reliance on ancestor height for embedded cases
  • ➕ Can more directly express “fill my containing viewport/container”
  • ➖ Higher risk of layering/scroll issues inside WP admin
  • ➖ More complex interaction with existing flex/overflow layout

Recommendation: Current approach is the best fit: it reuses the established embedded/standalone boundary (isEmbedded()), keeps the standalone behavior unchanged, and fixes the WP admin overflow without introducing WP-specific imports or more invasive layout changes.

Files changed (1) +9 / -2

Bug fix (1) +9 / -2
App.svelteConditionally size the root shell for embedded vs standalone hosts +9/-2

Conditionally size the root shell for embedded vs standalone hosts

• Imports and evaluates 'isEmbedded()' once at startup, then switches the root container from 'w-screen h-screen' to 'w-full h-full' when embedded. Adds inline documentation explaining why viewport units overflow inside WP admin’s document flow container.

configurator/src/App.svelte

@qodo-code-review

qodo-code-review Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 6 rules

Grey Divider


Action required

1. Embedded check too narrow ✓ Resolved 🐞 Bug ≡ Correctness
Description
App.svelte now switches to w-full h-full only when isEmbedded() is true, but isEmbedded()
returns true only when window.slashedApp.rest.url exists. If the WP host provides
window.slashedApp without rest.url (allowed by typings and already treated as “embedded
boundary” by loadInitialOverrides()), the root still renders w-screen h-screen and can
overflow/clamp incorrectly inside WP admin.
Code

configurator/src/App.svelte[242]

+<div class="{embedded ? 'w-full h-full' : 'w-screen h-screen'} flex flex-col overflow-hidden bg-[#0a0a0f] text-slate-200 font-sans">
Relevance

⭐⭐⭐ High

PR443 docs/loadInitialOverrides use window.slashedApp as embed boundary; current isEmbedded() uses
rest.url, mismatch likely addressed.

PR-#443

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
App.svelte’s new sizing decision depends on isEmbedded(), but isEmbedded() is implemented as a
REST URL check. Meanwhile, both the module docs and loadInitialOverrides() treat
window.slashedApp presence (even without rest) as the embedded-mode boundary, and the global
typing marks rest as optional—so these predicates can diverge in real boots.

configurator/src/App.svelte[34-42]
configurator/src/App.svelte[242-242]
configurator/src/lib/persistence.ts[8-11]
configurator/src/lib/persistence.ts[172-200]
configurator/src/vite-env.d.ts[5-15]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The UI layout toggle in `App.svelte` uses `isEmbedded()` to decide between viewport sizing (`w-screen h-screen`) and container sizing (`w-full h-full`). However, `isEmbedded()` currently checks only for `window.slashedApp.rest.url`, while other code paths treat any `window.slashedApp` presence as the embedded boundary.

This can misclassify some WP-embedded boots as standalone for layout, keeping the viewport-sized root and reintroducing the overflow/clipping in WP admin.

### Issue Context
- `loadInitialOverrides()` treats any `wpBoot()` (i.e., any `window.slashedApp`) as embedded boundary.
- `Window.slashedApp.rest` is optional in typings.
- `App.svelte` now uses `isEmbedded()` to choose sizing classes.

### Fix Focus Areas
- configurator/src/lib/persistence.ts[172-200]
- configurator/src/App.svelte[34-42]
- configurator/src/App.svelte[242-242]

### Suggested fix
Option A (recommended):
1. Add a new exported predicate for “WP host present”, e.g. `export function hasWpBoot(): boolean { return Boolean(wpBoot()); }`.
2. In `App.svelte`, use `hasWpBoot()` for sizing, while keeping `isEmbedded()` semantics as “REST persistence available”.

Option B:
- Redefine `isEmbedded()` to match the boundary used by `loadInitialOverrides()` (return `Boolean(wpBoot())`), and if needed introduce a separate helper like `hasWpRest()` for persistence-specific checks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread configurator/src/App.svelte
claude added 2 commits June 30, 2026 23:25
isEmbedded() specifically checks REST persistence availability
(window.slashedApp.rest.url), but a host can mount window.slashedApp
without configuring rest (it's optional in the type). Layout sizing
needs the broader boundary loadInitialOverrides() already uses — any
host-mounted boot — so add hasWpBoot() and use that for the
w-full/h-full vs w-screen/h-screen decision instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
Status families (success/warning/info/danger) have no numeric palette
scale in the framework — core/tokens.css only derives -subtle, -muted,
and -strong variants for them, unlike brand families which get a real
11-step -50..-950 ramp. The Status colors section was unconditionally
rendering an 11-swatch strip using the brand mix-curve formula anyway,
implying tokens like --sf-color-success-500 that don't exist.

Replace it with a preview of the four values that actually exist
(resolved color, subtle, muted, strong), resolved from the real CSS
custom properties via the existing themed-probe resolver instead of
re-implementing the formulas in JS.

Also add brief in-panel notes clarifying two behaviors that are
correct-but-surprising, not bugs: brand tints/shades intentionally mix
toward Base/Text so editing those shifts every family's ramp, and
Base's ramp uses the same lightness steps in light and dark mode so it
looks similar until you add chroma. And upgrade palette swatches'
hover state from a slow native title tooltip to an instant floating
label with the token name.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
@coderabbitai coderabbitai Bot added the codex label Jul 1, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@configurator/src/components/panels/ColorsPanel.svelte`:
- Around line 453-459: The swatchTip snippet in ColorsPanel.svelte renders a
visual-only hover label that is still exposed to assistive tech because it only
uses opacity-based hiding. Update the tooltip span in swatchTip(name) to be
aria-hidden so the floating label stays purely decorative and does not add
duplicate text to screen readers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 05a55ad2-1467-4f66-82f1-475e1ebc9f68

📥 Commits

Reviewing files that changed from the base of the PR and between 894cca7 and dbdf898.

📒 Files selected for processing (3)
  • configurator/src/App.svelte
  • configurator/src/components/panels/ColorsPanel.svelte
  • configurator/src/lib/persistence.ts

Comment thread configurator/src/components/panels/ColorsPanel.svelte
opacity-0/group-hover:opacity-100 only visually hides the floating
tooltip label — it stayed in the accessibility tree, so screen readers
would announce duplicate text for every palette swatch. Mark it
aria-hidden since it's a purely decorative visual affordance (the
title attribute already carries the accessible name).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants