Skip to content

Fix share link to use config code instead of current URL - #562

Merged
jackgranatowski merged 2 commits into
mainfrom
claude/mobile-configurator-ui-tp5nhk
Jul 7, 2026
Merged

jackgranatowski merged 2 commits into
mainfrom
claude/mobile-configurator-ui-tp5nhk

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

The share button now generates a proper shareable link using the current overrides as a config code, rather than copying the raw page URL. This is especially important for embedded hosts (like the WordPress plugin) where the current page is a logged-in admin screen, not something worth sharing.

Key Changes

  • StudioHeader: Updated handleShare() to call buildShareUrl(overrides, getShareBaseUrl()) instead of copying window.location.href directly
  • StudioHeader: Added overrides prop to component signature and passed it through from App.svelte
  • StudioHeader: Applied responsive design improvements to the header toolbar:
    • Added shrink-0 to all buttons and dividers to prevent flex shrinking on narrow screens
    • Made save button text responsive with hidden sm:inline (icon always visible, text hidden on mobile)
    • Added horizontal scroll container with hidden scrollbars for the button group
    • Reduced horizontal padding on save button for mobile (px-2 sm:px-3)
  • ExportPanel: Updated handleCopyLink() to use buildShareUrl(overrides, getShareBaseUrl())
  • persistence.ts: Added getShareBaseUrl() function that returns the host's public configurator URL when embedded (e.g., from WordPress plugin settings), or undefined in standalone mode
  • persistence.ts: Extended SlashedAppBoot interface to include pluginSettings with optional configurator_url
  • vite-env.d.ts: Added pluginSettings type definition to window.slashedApp
  • Tests: Added comprehensive test coverage for the share link functionality, including:
    • Verification that copied links contain config code (#c=)
    • Verification that embedded hosts use the correct base URL from plugin settings

Implementation Details

The fix ensures that:

  1. Share links always carry the current override state as a config code in the URL hash
  2. Embedded hosts (WordPress plugin, etc.) point to their public configurator URL instead of the admin page
  3. Standalone mode continues to use the current page URL as the base (via buildShareUrl's fallback)
  4. The header remains usable on mobile devices with responsive text hiding and proper flex shrinking

https://claude.ai/code/session_01QTPpAk2ynC74tuUAsVpYNf

Summary by CodeRabbit

  • New Features

    • Shareable links now respect the configured app URL when available, while still including the current configuration in the link.
    • The header now keeps action buttons usable in tighter spaces with improved horizontal scrolling.
  • Bug Fixes

    • Share links no longer rely on the current page URL, making copied links more consistent.
    • The reset action is disabled more clearly when there are no changes to undo.

The header's button cluster (Save…Theme toggle) had no shrink/scroll
guard, so on narrow viewports the rightmost buttons — including the
light/dark toggle — overflowed off-screen with no way to reach them.
Made the cluster a shrink-safe, scrollable strip and dropped the Save
button's text label below `sm` to reduce the common-case footprint.

Also fixed the header's Share button: it copied the raw current page
URL instead of building a proper share link, so shared configs never
carried the override code. It now uses buildShareUrl() like the Export
panel already does, and both share paths use a new getShareBaseUrl()
so embedded hosts (e.g. the WP plugin, which persists overrides via
REST instead of the URL hash) point the link at their public
standalone configurator instead of a logged-in admin URL.

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

coderabbitai Bot commented Jul 7, 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: 41 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: 991146d3-d215-40f1-a238-e29ef4fe9bf5

📥 Commits

Reviewing files that changed from the base of the PR and between b6f1f20 and d96eba6.

📒 Files selected for processing (2)
  • configurator/src/lib/persistence.ts
  • configurator/tests/persistence.test.js
📝 Walkthrough

Walkthrough

This PR introduces a getShareBaseUrl() helper and pluginSettings.configurator_url typing to support embedded WordPress plugin contexts, wires this into share-link generation in StudioHeader and ExportPanel via an added overrides prop, adjusts header layout/styling, and updates tests.

Changes

Embedded Share Base URL Support

Layer / File(s) Summary
Share base URL helper and typings
configurator/src/lib/persistence.ts, configurator/src/vite-env.d.ts
Adds pluginSettings?: { configurator_url?: string } to SlashedAppBoot and Window.slashedApp, and exports getShareBaseUrl() returning the trimmed non-empty URL or undefined.
StudioHeader share link wiring
configurator/src/components/shell/StudioHeader.svelte, configurator/src/App.svelte
Adds overrides prop, imports buildShareUrl/getShareBaseUrl, and updates handleShare to build and copy a share URL from overrides and the base URL; App.svelte passes overrides into StudioHeader.
ExportPanel share link update
configurator/src/components/panels/ExportPanel.svelte
Imports getShareBaseUrl and passes its result into buildShareUrl when generating the shareable link.
Header layout and button styling
configurator/src/components/shell/StudioHeader.svelte
Wraps action buttons in a horizontally scrollable container, adds shrink-0 to several buttons/separators, and adds disabled styling to the reset button.
Header tests for share link and overrides
configurator/tests-components/header.test.js
Adds overrides to baseProps, token-driven test setup, and new tests validating clipboard copy behavior with and without an embedded configurator_url.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant App
  participant StudioHeader
  participant persistence
  participant Clipboard

  App->>StudioHeader: pass overrides prop
  StudioHeader->>persistence: getShareBaseUrl()
  persistence-->>StudioHeader: configurator_url or undefined
  StudioHeader->>StudioHeader: buildShareUrl(overrides, baseUrl)
  StudioHeader->>Clipboard: copy share URL
Loading

Suggested labels: codex

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: share links now use the config code instead of the current page URL.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/mobile-configurator-ui-tp5nhk

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.

@coderabbitai coderabbitai Bot added the codex label Jul 7, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix share link to use config code instead of raw page URL

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Share button and Export panel now build a proper share URL via `buildShareUrl(overrides,
 getShareBaseUrl()) instead of copying window.location.href`.
• Added getShareBaseUrl() in persistence.ts so embedded hosts (e.g. WordPress plugin) point shares
 at their public configurator URL instead of a logged-in admin page.
• Extended SlashedAppBoot/window.slashedApp typings with pluginSettings.configurator_url.
• Made the header toolbar responsive: shrink-safe scrollable button strip, save button text hidden
 on mobile.
• Added test coverage for share-link config code and embedded base URL behavior.
Diagram

sequenceDiagram
  actor User
  participant Header as StudioHeader
  participant Export as ExportPanel
  participant Codec as "codec.buildShareUrl"
  participant Persist as "persistence.getShareBaseUrl"
  participant Boot as "window.slashedApp"
  User->>Header: click Share
  Header->>Persist: getShareBaseUrl()
  Persist->>Boot: read pluginSettings.configurator_url
  Boot-->>Persist: url or undefined
  Persist-->>Header: baseUrl
  Header->>Codec: buildShareUrl(overrides, baseUrl)
  Codec-->>Header: shareable URL with #c= code
  Header-->>User: copy to clipboard
  User->>Export: click Copy link
  Export->>Persist: getShareBaseUrl()
  Export->>Codec: buildShareUrl(overrides, baseUrl)
  Codec-->>Export: shareable URL
  Export-->>User: copy to clipboard
Loading
High-Level Assessment

The fix correctly reuses the existing buildShareUrl() abstraction (already used by ExportPanel) rather than duplicating URL-building logic in StudioHeader, and centralizes host-detection in a single getShareBaseUrl() helper alongside the existing wpBoot()/isEmbedded() pattern in persistence.ts. This is the natural, minimal-risk fix; no alternative architecture (e.g. a separate service, server-side redirect) offers meaningful benefit for this scope.

Files changed (6) +77 / -22

Enhancement (2) +16 / -0
App.sveltePass overrides prop to StudioHeader +1/-0

Pass overrides prop to StudioHeader

• Threads the 'overrides' state through to StudioHeader so its share handler can encode the current config into the share URL.

configurator/src/App.svelte

persistence.tsAdd getShareBaseUrl() and pluginSettings typing +15/-0

Add getShareBaseUrl() and pluginSettings typing

• Introduces getShareBaseUrl(), which reads pluginSettings.configurator_url from the WP boot payload to determine the public share base URL, falling back to undefined in standalone mode. Extends SlashedAppBoot with the new pluginSettings field.

configurator/src/lib/persistence.ts

Bug fix (2) +26 / -21
ExportPanel.svelteUse buildShareUrl + getShareBaseUrl for copy link +2/-1

Use buildShareUrl + getShareBaseUrl for copy link

• Replaces implicit base URL usage with an explicit call to getShareBaseUrl() so the export panel's copy-link button respects embedded host settings.

configurator/src/components/panels/ExportPanel.svelte

StudioHeader.svelteFix share button to build config-code URL; responsive toolbar +24/-20

Fix share button to build config-code URL; responsive toolbar

• handleShare() now calls buildShareUrl(overrides, getShareBaseUrl()) instead of copying window.location.href; overrides added as a required prop. Also applies shrink-0/scroll/responsive text classes to the button toolbar so it fits on narrow viewports.

configurator/src/components/shell/StudioHeader.svelte

Tests (1) +34 / -1
header.test.jsAdd share link tests for config code and embedded base URL +34/-1

Add share link tests for config code and embedded base URL

• Adds tests verifying the share button copies a URL containing a config code (#c=) and that embedded hosts use the configured public configurator URL from pluginSettings.

configurator/tests-components/header.test.js

Other (1) +1 / -0
vite-env.d.tsAdd pluginSettings type to window.slashedApp +1/-0

Add pluginSettings type to window.slashedApp

• Declares the pluginSettings.configurator_url field on the global window.slashedApp typing to match the new boot payload shape.

configurator/src/vite-env.d.ts

@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/lib/persistence.ts`:
- Around line 189-202: getShareBaseUrl currently returns any non-empty
configurator_url, but buildShareUrl later requires an absolute URL and will fail
silently when StudioHeader.handleShare or ExportPanel.handleCopyLink catch the
error. Tighten getShareBaseUrl to validate the
wpBoot().pluginSettings.configurator_url value shape before returning it, only
allowing absolute URLs with a valid protocol/host and otherwise returning
undefined so the existing fallback path can be used. Use the getShareBaseUrl and
buildShareUrl flow as the place to fix this misconfiguration handling.
🪄 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: b4399162-b3ac-49ba-9112-cee6c1758831

📥 Commits

Reviewing files that changed from the base of the PR and between e83b1ff and b6f1f20.

📒 Files selected for processing (6)
  • configurator/src/App.svelte
  • configurator/src/components/panels/ExportPanel.svelte
  • configurator/src/components/shell/StudioHeader.svelte
  • configurator/src/lib/persistence.ts
  • configurator/src/vite-env.d.ts
  • configurator/tests-components/header.test.js

Comment thread configurator/src/lib/persistence.ts
@qodo-code-review

qodo-code-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 12 rules

Grey Divider


Action required

1. Invalid share base URL ✓ Resolved 🐞 Bug ☼ Reliability
Description
getShareBaseUrl() returns pluginSettings.configurator_url verbatim, but buildShareUrl() calls
new URL(baseUrl), which throws on malformed or relative values (e.g. /configurator). The
resulting exception is swallowed by the share/copy handlers, so users get no link copied and no
feedback in embedded hosts.
Code

configurator/src/lib/persistence.ts[R198-201]

+export function getShareBaseUrl(): string | undefined {
+  const url = wpBoot()?.pluginSettings?.configurator_url;
+  return url && url.trim() !== "" ? url : undefined;
+}
Relevance

⭐⭐ Medium

Accepted surfacing WP save failures, but rejected adding explicit error signals elsewhere; URL
validation unclear.

PR-#443
PR-#366

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
getShareBaseUrl() passes through the configured string, while buildShareUrl() uses `new
URL(baseUrl)` which throws on invalid/relative URLs; both share-link entry points wrap the operation
in try { ... } catch {} and therefore hide the failure from users.

configurator/src/lib/persistence.ts[189-201]
configurator/src/lib/codec.ts[278-285]
configurator/src/components/shell/StudioHeader.svelte[36-45]
configurator/src/components/panels/ExportPanel.svelte[25-32]

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

## Issue description
`getShareBaseUrl()` can return a relative or otherwise invalid URL string from host boot data. `buildShareUrl()` uses `new URL(baseUrl)` (no base), which throws on invalid/relative input; the callers swallow the error, so share link copy silently fails.

## Issue Context
This PR intentionally routes share-link generation through `buildShareUrl(overrides, getShareBaseUrl())` for embedded hosts. That makes `pluginSettings.configurator_url` a new runtime input to the URL parser.

## Fix Focus Areas
- configurator/src/lib/persistence.ts[189-201]
- configurator/src/lib/codec.ts[278-285]
- configurator/src/components/shell/StudioHeader.svelte[36-45]
- configurator/src/components/panels/ExportPanel.svelte[25-32]

## Recommended fix
- Harden URL construction so relative/malformed `configurator_url` cannot break sharing:
 - Option A (preferred): in `buildShareUrl()`, construct with a base: `new URL(baseUrlOverride ?? fallbackUrl, fallbackUrl)` and/or wrap parsing in try/catch and fall back to `fallbackUrl`.
 - Option B: in `getShareBaseUrl()`, resolve/normalize via `new URL(trimmed, window.location.href)` and return `undefined` if parsing fails.
- Consider adding a minimal `console.warn` when falling back so failures aren’t completely silent (optional but helpful).

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



Remediation recommended

2. Unsafe URL schemes copied ✓ Resolved 🐞 Bug ⛨ Security
Description
configurator_url is accepted without protocol validation, so a host can supply
javascript:/data:/other non-http(s) schemes that will be copied as a “share link”. This is
primarily a misconfiguration/phishing footgun, since the app will faithfully propagate the unsafe
scheme into the copied URL.
Code

configurator/src/lib/persistence.ts[R198-201]

+export function getShareBaseUrl(): string | undefined {
+  const url = wpBoot()?.pluginSettings?.configurator_url;
+  return url && url.trim() !== "" ? url : undefined;
+}
Relevance

⭐⭐⭐ High

Team repeatedly accepts hardening against untrusted inputs; scheme allowlist fits prior security
fixes.

PR-#462
PR-#427

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
getShareBaseUrl() returns whatever string is provided in boot data, and buildShareUrl()
preserves the scheme by parsing it into a URL and returning toString(). There is no allowlist of
safe protocols in between.

configurator/src/lib/persistence.ts[198-201]
configurator/src/lib/codec.ts[278-285]

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 host-controlled `pluginSettings.configurator_url` is not validated. If it contains a non-http(s) scheme (e.g. `javascript:`), the app will copy that scheme into the generated share link.

## Issue Context
Even though the app only copies the string to the clipboard, users may paste/open it, so it’s worth preventing obviously unsafe schemes.

## Fix Focus Areas
- configurator/src/lib/persistence.ts[189-201]
- configurator/src/lib/codec.ts[278-285]

## Recommended fix
- When parsing/normalizing the base URL (either in `getShareBaseUrl()` or `buildShareUrl()`), explicitly allow only `http:` and `https:` protocols.
- If the parsed protocol is not allowed (or parsing fails), return `undefined` / fall back to `window.location.href` so share links remain functional.

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


Grey Divider

Qodo Logo

Comment thread configurator/src/lib/persistence.ts
Comment on lines +198 to +201
export function getShareBaseUrl(): string | undefined {
const url = wpBoot()?.pluginSettings?.configurator_url;
return url && url.trim() !== "" ? url : undefined;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Unsafe url schemes copied 🐞 Bug ⛨ Security

configurator_url is accepted without protocol validation, so a host can supply
javascript:/data:/other non-http(s) schemes that will be copied as a “share link”. This is
primarily a misconfiguration/phishing footgun, since the app will faithfully propagate the unsafe
scheme into the copied URL.
Agent Prompt
## Issue description
The host-controlled `pluginSettings.configurator_url` is not validated. If it contains a non-http(s) scheme (e.g. `javascript:`), the app will copy that scheme into the generated share link.

## Issue Context
Even though the app only copies the string to the clipboard, users may paste/open it, so it’s worth preventing obviously unsafe schemes.

## Fix Focus Areas
- configurator/src/lib/persistence.ts[189-201]
- configurator/src/lib/codec.ts[278-285]

## Recommended fix
- When parsing/normalizing the base URL (either in `getShareBaseUrl()` or `buildShareUrl()`), explicitly allow only `http:` and `https:` protocols.
- If the parsed protocol is not allowed (or parsing fails), return `undefined` / fall back to `window.location.href` so share links remain functional.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d96eba6getShareBaseUrl() now allowlists only http:/https: schemes; a javascript:/data: value is rejected (warned + undefined) rather than propagated into a copied link.


Generated by Claude Code

getShareBaseUrl() returned pluginSettings.configurator_url verbatim, but
buildShareUrl() feeds it to new URL(baseUrl), which throws on a relative
or malformed value — and both share/copy handlers swallow the exception,
so a misconfigured host would make "Copy link" silently do nothing. It
also propagated any scheme, so a javascript:/data: value could be copied
as a "share link". Now parse-and-validate the URL, require an http(s)
scheme, and return undefined (with a warning) otherwise so buildShareUrl
falls back to the current page URL.

Addresses review feedback on #562.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QTPpAk2ynC74tuUAsVpYNf
@jackgranatowski
jackgranatowski merged commit bc2f5ce into main Jul 7, 2026
13 checks passed
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