feat: tracking components, SDK option sync, pause toggle#9
Conversation
Adds three Framer code components (TrackClick, TrackSubmit, TrackView) generated into Databuddy.tsx with property controls for sidebar config. Syncs script attributes with current SDK options (debug, disabled, samplingRate, skip/maskPatterns, usePixel, ignoreBotDetection, customApiUrl). Adds Pause/Resume toggle in the footer and a Custom Events section that previews the available trackers and installs them into the project on demand.
📝 WalkthroughWalkthroughThe PR refactors styling classes, adds support for filtering and debug configuration options, introduces generalized collapsible components replacing self-hosted patterns, and implements permission-aware component file management through Framer API integrations for tracking component installation and dashboard menu customization. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/App
participant App as App.tsx
participant Perms as useIsAllowedTo
participant Components as components.ts
participant Framer as Framer API
participant Settings as settings.ts
User->>App: Click Install Components
App->>Perms: Check "setPluginData" permission
Perms-->>App: Permission granted/denied
alt Permission Denied
App->>User: Show disabled button with tooltip
else Permission Granted
App->>Components: installComponents()
Components->>Components: Locate/create Databuddy.tsx
Components->>Framer: Create or update CodeFile
Framer-->>Components: CodeFile result
Components->>Settings: Save installation state
Settings->>Framer: setPluginData with permission check
Framer-->>Settings: Persisted
Components-->>App: InstallResult {status, componentCount}
App->>User: Navigate to component + show success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/settings.ts (1)
43-51:⚠️ Potential issue | 🟠 MajorSilent failure when
setPluginDatapermission is missing.
saveSettingresolves successfully when the user lackssetPluginDatapermission, but nothing is persisted. Every caller inApp.tsx(handleSettingChange,handleClientIdChange,handleDebugChange,handleDisabledChange,saveApiUrl, the pattern/sampling handlers, etc.) then updates local React state and callsupdateScript(), which reads stale values back fromframer.getPluginData. Net effect: the UI appears to change, the Pause toggle visually flips, but the injected<script>tag is not re-synced — exactly the opposite of what the PR's permission gating is trying to achieve.Consider returning a success flag (or throwing) so callers can short-circuit and notify the user, e.g.:
♻️ Proposed fix
export async function saveSetting( id: string, value: string | null -): Promise<void> { - if (!framer.isAllowedTo("setPluginData")) { - return; - } - await framer.setPluginData(`${SETTING_PREFIX}${id}`, value); +): Promise<boolean> { + if (!framer.isAllowedTo("setPluginData")) { + return false; + } + await framer.setPluginData(`${SETTING_PREFIX}${id}`, value); + return true; }Callers in
App.tsxshould then skip the local state/updateScript()update (or surface aframer.notifywarning) when persistence didn't happen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/settings.ts` around lines 43 - 51, saveSetting currently silently returns when framer.isAllowedTo("setPluginData") is false which causes callers to proceed as if persistence succeeded; change saveSetting(id, value) to indicate failure (either throw an Error or return a boolean) so callers can short-circuit persistence-dependent updates: inside saveSetting (referencing framer.isAllowedTo, framer.setPluginData, and SETTING_PREFIX) return false or throw when permission is missing and return true after successful framer.setPluginData; then update callers in App.tsx (handleSettingChange, handleClientIdChange, handleDebugChange, handleDisabledChange, saveApiUrl and the pattern/sampling handlers) to check the result and skip local state/updateScript() or surface a framer.notify warning when saveSetting failed.src/App.tsx (1)
289-320:⚠️ Potential issue | 🟡 MinorPattern/sampling inputs persist and regenerate the script on every keystroke.
handleSamplingRateChange,handleSkipPatternsChange, andhandleMaskPatternsChangecallsaveSetting+updateScript()on everyonChange. Two concerns:
- Intermediate invalid JSON reaches the published script. While a user types
["/admin"the script is rewritten with that partial value as adata-skip-patternsattribute (seesrc/lib/script.tsline 86-91). Only the last keystroke is valid.framer.setCustomCode+framer.setPluginDatarun on every keystroke, where a debounce or anonBlursave (like the pattern you already use forsaveDashboardUrl/saveApiUrlat lines 246-265) would be more efficient and avoid flooding Framer's custom-code subscribers.Recommend switching these three to the same
onBlur+ local-state pattern used by the URL fields, and optionally validating the JSON on blur before persisting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 289 - 320, The three handlers handleSamplingRateChange, handleSkipPatternsChange, and handleMaskPatternsChange currently persist on every keystroke; change them to update only local component state on input change (call setSamplingRate/setSkipPatterns/setMaskPatterns) and move saveSetting + updateScript calls into corresponding onBlur handlers so persistence and framer.setCustomCode/framer.setPluginData only run when the field loses focus (and only if isInstalled). Additionally, onBlur validate JSON for skipPatterns and maskPatterns (and coerce/validate samplingRate) before calling saveSetting; if invalid, do not persist and surface an error. Keep saveSetting and updateScript usage but invoke them from the new onBlur handlers and ensure they receive the validated value (value || null).
🧹 Nitpick comments (4)
src/components/ui/label.tsx (1)
22-25: Minor: wrapping arbitraryReactNodein<p>may produce invalid HTML.
tooltipis typed asReactNode, so a caller could pass a block element (e.g.<div>,<ul>) and end up nested inside a<p>, which React will warn about and browsers auto-close. All current call sites inApp.tsxpass strings, so this is latent rather than active.♻️ Proposed fix
- <TooltipContent className="tooltip-large" side="top"> - <p>{tooltip}</p> - </TooltipContent> + <TooltipContent className="tooltip-large" side="top"> + {typeof tooltip === "string" ? <p>{tooltip}</p> : tooltip} + </TooltipContent>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/label.tsx` around lines 22 - 25, The TooltipContent currently wraps the ReactNode prop tooltip in a <p>, which can produce invalid HTML if callers pass block elements; update the Label component so TooltipContent renders tooltip directly (i.e., remove the hardcoded <p>), or alternatively conditionally wrap only when typeof tooltip === 'string' (or React.isValidElement check) so blocks like <div> or <ul> are not nested inside a paragraph; update the JSX around TooltipContent (and keep TooltipTrigger/InfoIcon usage unchanged) and adjust typings/comments if needed.src/lib/script.ts (1)
21-31:isValidHttpUrl("")returnstrue— document or rename.Returning
truefor the empty string is intentional (empty = use default), but the name implies pure validation. Callers outsideApp.tsx's blur handlers could be surprised. A short JSDoc note or rename toisValidHttpUrlOrEmptywould make the contract explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/script.ts` around lines 21 - 31, The function isValidHttpUrl currently returns true for the empty string which is intentional but mismatches its name; either add a JSDoc comment on isValidHttpUrl clarifying that empty string is treated as valid (meaning "use default"), or rename the function to isValidHttpUrlOrEmpty and update all call sites (e.g., the blur handlers that rely on this behavior) to use the new name; ensure the chosen change is applied consistently and the JSDoc (if added) states the exact contract: empty string allowed, otherwise must be http/https.src/lib/overrides.ts (1)
5-145: Consider movingCOMPONENT_FILE_CONTENTto a separate asset.Embedding ~140 lines of TSX as a template literal inside a
.tsfile loses type-checking, lint, and editor tooling on arguably the most user-facing code in the plugin — and a bad edit here silently ships to every user'sDatabuddy.tsx. Two cleaner options:
- Keep the code in a real
src/lib/component-template.tsxfile and import it as a raw string via Vite's?rawimport (e.g.,import componentContent from './component-template.tsx?raw').- Or keep it inline but add a unit test that at minimum runs the string through a TS parser to catch regressions before shipping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/overrides.ts` around lines 5 - 145, COMPONENT_FILE_CONTENT currently embeds ~140 lines of TSX as a template string which loses type/lint tooling; move that TSX into a real asset and then replace the inline string with an import of the raw text (assigning it back to COMPONENT_FILE_CONTENT) via your bundler's raw import (e.g., import componentContent from './component-template.tsx?raw' and set COMPONENT_FILE_CONTENT = componentContent), or if you prefer to keep it inline add a unit test that parses COMPONENT_FILE_CONTENT with the TypeScript parser to catch syntax/type regressions before release; update any references expecting the original string (COMPONENT_FILE_CONTENT) so TrackClick / TrackSubmit / TrackView consumers continue to work.src/App.css (1)
121-143: Prevent component rows from overflowing in narrow plugin panels.The list row uses
display: flex, but neither text side setsmin-width: 0; long component names/descriptions can force the row wider than the Framer plugin sidebar instead of truncating or wrapping.Proposed CSS adjustment
.components-list > li { display: flex; align-items: center; justify-content: space-between; gap: 12px; padding: 8px 12px; font-size: 12px; } .components-name { color: var(--framer-color-text); font-weight: 500; + min-width: 0; } .components-desc { color: var(--framer-color-text-tertiary); font-size: 11px; text-align: right; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.css` around lines 121 - 143, The flex children in .components-list > li (.components-name and .components-desc) lack min-width constraints, allowing long text to force the row wider; update the CSS for .components-name and .components-desc to set min-width: 0 and apply overflow handling (overflow: hidden; text-overflow: ellipsis; white-space: nowrap or allow wrapping as desired) so long names/descriptions truncate instead of overflowing the plugin panel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 188-201: The menu currently shows "Open Tracking Components" but
wired to handleInstallComponents which calls installComponents() and overwrites
Databuddy.tsx; change the menu logic so when componentsInstalled is true it uses
a new open-only handler (e.g., handleOpenComponents) that only opens/navigates
to the existing Databuddy.tsx without calling installComponents() or
existing.setFileContent, and add a separate menu entry "Reinstall/Update
Tracking Components" that calls handleInstallComponents to perform the
overwrite; update references to componentsInstalled, handleInstallComponents,
handleRemoveComponents, installComponents(), COMPONENT_FILE_CONTENT and
Databuddy.tsx to ensure the new handler only opens and never writes the file.
In `@src/lib/overrides.ts`:
- Around line 88-114: In TrackView, the fired ref is never reset so changing
props.eventName or props.properties won't allow the observer to fire again;
update the effect so that when the identity-defining props (props.eventName and
props.properties, and threshold if desired) change you reset fired.current =
false before creating the IntersectionObserver (or, if the intended behavior is
“fire once per mount”, remove those props from the effect deps and read them via
refs instead) to ensure the observer can fire for the new configuration.
- Around line 176-182: The removeComponents function can throw for view-only
users because it calls existing.remove() without checking permissions; update
removeComponents to first call framer.isAllowedTo("CodeFile.remove") and
early-return (resolve/return void) when false, or alternatively add a permission
gate in App.tsx using useIsAllowedTo("CodeFile.remove") to disable the Remove
button and menu path (the latter mirrors the existing
canCreateComponents/installComponents pattern); ensure references to
findComponentFile and handleRemoveComponents still work and surface an
informative result or no-op instead of attempting existing.remove() when not
allowed.
In `@src/lib/script.ts`:
- Around line 61-98: The interpolated attribute values (clientId, customApiUrl,
skipPatterns, maskPatterns) are not escaped and can break the <script> tag or
allow attribute injection; add a small helper (e.g., escapeHtmlAttr) and call it
before interpolation: escape values used inside double-quoted attributes
(clientId, customApiUrl, any values added to attrString) by replacing " with
" (and & with &), and escape values used inside single-quoted
attributes (skipPatterns, maskPatterns) by replacing ' with ' (and & with
&); update the template in the return expression to use the escaped versions
(refer to clientId, customApiUrl, skipPatterns, maskPatterns) and also consider
validating skipPatterns/maskPatterns as JSON at save time (same logic you use in
App.tsx) to avoid shipping malformed JSON.
---
Outside diff comments:
In `@src/App.tsx`:
- Around line 289-320: The three handlers handleSamplingRateChange,
handleSkipPatternsChange, and handleMaskPatternsChange currently persist on
every keystroke; change them to update only local component state on input
change (call setSamplingRate/setSkipPatterns/setMaskPatterns) and move
saveSetting + updateScript calls into corresponding onBlur handlers so
persistence and framer.setCustomCode/framer.setPluginData only run when the
field loses focus (and only if isInstalled). Additionally, onBlur validate JSON
for skipPatterns and maskPatterns (and coerce/validate samplingRate) before
calling saveSetting; if invalid, do not persist and surface an error. Keep
saveSetting and updateScript usage but invoke them from the new onBlur handlers
and ensure they receive the validated value (value || null).
In `@src/lib/settings.ts`:
- Around line 43-51: saveSetting currently silently returns when
framer.isAllowedTo("setPluginData") is false which causes callers to proceed as
if persistence succeeded; change saveSetting(id, value) to indicate failure
(either throw an Error or return a boolean) so callers can short-circuit
persistence-dependent updates: inside saveSetting (referencing
framer.isAllowedTo, framer.setPluginData, and SETTING_PREFIX) return false or
throw when permission is missing and return true after successful
framer.setPluginData; then update callers in App.tsx (handleSettingChange,
handleClientIdChange, handleDebugChange, handleDisabledChange, saveApiUrl and
the pattern/sampling handlers) to check the result and skip local
state/updateScript() or surface a framer.notify warning when saveSetting failed.
---
Nitpick comments:
In `@src/App.css`:
- Around line 121-143: The flex children in .components-list > li
(.components-name and .components-desc) lack min-width constraints, allowing
long text to force the row wider; update the CSS for .components-name and
.components-desc to set min-width: 0 and apply overflow handling (overflow:
hidden; text-overflow: ellipsis; white-space: nowrap or allow wrapping as
desired) so long names/descriptions truncate instead of overflowing the plugin
panel.
In `@src/components/ui/label.tsx`:
- Around line 22-25: The TooltipContent currently wraps the ReactNode prop
tooltip in a <p>, which can produce invalid HTML if callers pass block elements;
update the Label component so TooltipContent renders tooltip directly (i.e.,
remove the hardcoded <p>), or alternatively conditionally wrap only when typeof
tooltip === 'string' (or React.isValidElement check) so blocks like <div> or
<ul> are not nested inside a paragraph; update the JSX around TooltipContent
(and keep TooltipTrigger/InfoIcon usage unchanged) and adjust typings/comments
if needed.
In `@src/lib/overrides.ts`:
- Around line 5-145: COMPONENT_FILE_CONTENT currently embeds ~140 lines of TSX
as a template string which loses type/lint tooling; move that TSX into a real
asset and then replace the inline string with an import of the raw text
(assigning it back to COMPONENT_FILE_CONTENT) via your bundler's raw import
(e.g., import componentContent from './component-template.tsx?raw' and set
COMPONENT_FILE_CONTENT = componentContent), or if you prefer to keep it inline
add a unit test that parses COMPONENT_FILE_CONTENT with the TypeScript parser to
catch syntax/type regressions before release; update any references expecting
the original string (COMPONENT_FILE_CONTENT) so TrackClick / TrackSubmit /
TrackView consumers continue to work.
In `@src/lib/script.ts`:
- Around line 21-31: The function isValidHttpUrl currently returns true for the
empty string which is intentional but mismatches its name; either add a JSDoc
comment on isValidHttpUrl clarifying that empty string is treated as valid
(meaning "use default"), or rename the function to isValidHttpUrlOrEmpty and
update all call sites (e.g., the blur handlers that rely on this behavior) to
use the new name; ensure the chosen change is applied consistently and the JSDoc
(if added) states the exact contract: empty string allowed, otherwise must be
http/https.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eaa3eaf-6370-4bc4-b9dc-98a35be3137a
📒 Files selected for processing (9)
src/App.csssrc/App.tsxsrc/components/ui/label.tsxsrc/framer.csssrc/hooks/use-overrides.tssrc/lib/overrides.tssrc/lib/script.tssrc/lib/settings.tssrc/util/integration.ts
| { | ||
| label: componentsInstalled | ||
| ? "Open Tracking Components" | ||
| : "Install Tracking Components", | ||
| onAction: handleInstallComponents, | ||
| }, | ||
| ...(componentsInstalled | ||
| ? [ | ||
| { | ||
| label: "Remove Tracking Components", | ||
| onAction: handleRemoveComponents, | ||
| } as const, | ||
| ] | ||
| : []), |
There was a problem hiding this comment.
"Open Tracking Components" menu item silently overwrites user edits.
When componentsInstalled is true, the menu shows "Open Tracking Components" but its onAction is handleInstallComponents → installComponents(), which calls existing.setFileContent(COMPONENT_FILE_CONTENT) before navigating (see src/lib/overrides.ts lines 163-166). Any customization a user made to Databuddy.tsx is lost the moment they choose "Open". Users reasonably expect "Open" to only navigate.
Suggest splitting into an open-only action when the file already exists:
♻️ Proposed fix
{
label: componentsInstalled
? "Open Tracking Components"
: "Install Tracking Components",
- onAction: handleInstallComponents,
+ onAction: componentsInstalled
+ ? async () => {
+ if (componentFile) await componentFile.navigateTo();
+ }
+ : handleInstallComponents,
},You can keep a separate "Reinstall/Update Tracking Components" entry for the overwrite path.
| export async function removeComponents(): Promise<void> { | ||
| const files = await framer.getCodeFiles(); | ||
| const existing = findComponentFile(files); | ||
| if (existing) { | ||
| await existing.remove(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the exact permission identifier for removing a code file in framer-plugin v3.0.0
rg -nP --type=ts -C2 "isAllowedTo\("
fd -i 'framer-plugin' node_modules 2>/dev/null | head
rg -nP -C2 "removeCodeFile|'createCodeFile'|\"createCodeFile\""Repository: databuddy-analytics/databuddy-framer
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Find handleRemoveComponents and understand the flow
rg -nP "handleRemoveComponents" --type=ts -A5 -B2Repository: databuddy-analytics/databuddy-framer
Length of output: 1492
🏁 Script executed:
#!/bin/bash
# Find where removeComponents is called and how the remove button is implemented
rg -nP "removeComponents|Remove button|remove.*button" --type=ts -A3 -B2Repository: databuddy-analytics/databuddy-framer
Length of output: 1041
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or type definitions for Framer plugin permissions
rg -nP "removeCodeFile|remove.*Code.*File" --type=ts
find . -name "*.d.ts" -o -name "framer*.ts" | head -20Repository: databuddy-analytics/databuddy-framer
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Get more context on canCreateComponents usage and see what other permissions exist
rg -nP "useIsAllowedTo\(" --type=ts -A1 | head -30Repository: databuddy-analytics/databuddy-framer
Length of output: 303
🏁 Script executed:
#!/bin/bash
# Check for any framer API calls that might document available permissions
rg -nP "framer\.(isAllowedTo|getCodeFiles|notify)" --type=ts -B2 -A2Repository: databuddy-analytics/databuddy-framer
Length of output: 3897
🏁 Script executed:
#!/bin/bash
# Look for error handling patterns and any documentation in comments
rg -nP "remove\(\)|Could not remove" --type=ts -B3 -A3Repository: databuddy-analytics/databuddy-framer
Length of output: 741
🏁 Script executed:
#!/bin/bash
# Check package.json or any type definitions for framer-plugin version and permissions
cat package.json 2>/dev/null | grep -A2 -B2 "framer"
head -50 src/vite-env.d.tsRepository: databuddy-analytics/databuddy-framer
Length of output: 476
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about available framer permissions
rg -nP "permission|Permission|PERMISSION" --type=ts -i | grep -i "remove\|create\|code" | head -20Repository: databuddy-analytics/databuddy-framer
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Check if there's any README or documentation about Framer permissions
find . -maxdepth 3 -type f \( -name "README*" -o -name "*.md" \) | xargs grep -l "permission\|removeCodeFile" 2>/dev/null | head -5Repository: databuddy-analytics/databuddy-framer
Length of output: 62
🌐 Web query:
Framer plugin v3 permissions API removeCodeFile
💡 Result:
In Framer Plugins API v3, the remove method on a CodeFile instance (accessed via framer.getCodeFile or createCodeFile) removes the code file from the project: await codeFile.remove;. This method is likely protected by permissions, as it modifies the project. Plugins must check user permissions using the Permissions API before calling it to avoid FramerPluginError. To check permissions for CodeFile.remove, use framer.isAllowedTo("CodeFile.remove"), which returns a boolean indicating if the user can execute it. For React plugins, use the useIsAllowedTo("CodeFile.remove") hook. For dynamic checks, framer.subscribeToIsAllowedTo("CodeFile.remove", callback). Example usage: if (framer.isAllowedTo("CodeFile.remove")) { await codeFile.remove; } else { // Handle insufficient permissions, e.g., show message } Permissions depend on the user's project role (e.g., Design or Content access). No explicit confirmation that CodeFile.remove is protected, but follows pattern for destructive class methods like "CollectionItem.remove". Methods starting with "get" are typically always allowed. If permissions are insufficient, a toast shows required access. Official docs: CodeFile.remove (framer.com/developers/reference/plugins-code-file-remove), Permissions guide (framer.com/developers/plugins-permissions).
Citations:
- 1: https://www.framer.com/developers/reference/plugins-code-file-remove
- 2: http://framer.com/developers/reference/plugins-is-allowed-to
- 3: https://www.framer.com/developers/plugins-permissions
- 4: https://www.framer.com/developers/reference/plugins-code-file
🏁 Script executed:
#!/bin/bash
# Verify the codebase isn't already using CodeFile.remove permission
rg -nP "CodeFile\." --type=ts
rg -nP "CodeFile\.remove" --type=tsRepository: databuddy-analytics/databuddy-framer
Length of output: 62
removeComponents does not gate on CodeFile.remove permission.
installComponents is protected at the App layer via canCreateComponents = useIsAllowedTo("createCodeFile"), but the remove button and menu simply call handleRemoveComponents without any equivalent check. If a view-only collaborator ends up with this button enabled (e.g. the Framer menu path, which never checks permissions), existing.remove() will throw and only show an "Could not remove file" toast.
Recommend either:
- Adding a
useIsAllowedTo("CodeFile.remove")check inApp.tsxmirroringcanCreateComponents, disabling the menu entry and Remove button accordingly, or - Early-returning from
removeComponents()with an informative result whenframer.isAllowedTo("CodeFile.remove")is false.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/overrides.ts` around lines 176 - 182, The removeComponents function
can throw for view-only users because it calls existing.remove() without
checking permissions; update removeComponents to first call
framer.isAllowedTo("CodeFile.remove") and early-return (resolve/return void)
when false, or alternatively add a permission gate in App.tsx using
useIsAllowedTo("CodeFile.remove") to disable the Remove button and menu path
(the latter mirrors the existing canCreateComponents/installComponents pattern);
ensure references to findComponentFile and handleRemoveComponents still work and
surface an informative result or no-op instead of attempting existing.remove()
when not allowed.
…ename overrides → components - Escape every value interpolated into the injected <script> tag. Skip/mask patterns previously used single-quoted attributes to permit JSON's double quotes; a literal single quote in user input would break out and inject arbitrary HTML/JS into the published site. - Coerce samplingRate to a number and only emit data-sampling-rate when it parses to a finite value in [0, 1). Fixes "1.0" / "1.00" leaking through the previous string compare against "1". - Validate customApiUrl with isValidHttpUrl before injection. - Rename src/lib/overrides.ts → src/lib/components.ts and src/hooks/use-overrides.ts → src/hooks/use-component-file.ts to match the implementation (Code Components, not Code Overrides). - Hold clientId in a ref inside the Open Dashboard menu action so the menu effect no longer rebinds on every keystroke.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.tsx (1)
286-317:⚠️ Potential issue | 🟠 MajorFiltering inputs write to Framer custom code on every keystroke.
handleSamplingRateChange,handleSkipPatternsChange, andhandleMaskPatternsChangeeach callsaveSettingand, when installed,updateScript()— which ends up inframer.setCustomCode({ html, location: "headEnd" })on every character typed. That's both a lot of unnecessary project writes and produces transient invalid values forskipPatterns/maskPatterns(e.g. while the user is mid-typing a JSON array, every intermediate state like[or["/admis persisted and shipped). You already adoptedonBlurfor the dashboard/API URL inputs (lines 536, 553) for the same reason — apply the same pattern here, or debounce.♻️ Suggested pattern (onBlur persistence for pattern inputs)
- const handleSkipPatternsChange = useCallback( - async (value: string) => { - setSkipPatterns(value); - await saveSetting("skipPatterns", value || null); - if (isInstalled) { - await updateScript(); - } - }, - [isInstalled] - ); + const saveSkipPatterns = useCallback(async () => { + await saveSetting("skipPatterns", skipPatterns || null); + if (isInstalled) await updateScript(); + }, [skipPatterns, isInstalled]);…then wire
onChange={(e) => setSkipPatterns(e.target.value)}+onBlur={saveSkipPatterns}on the input (same formaskPatterns, and optionallysamplingRate).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 286 - 317, The three handlers (handleSamplingRateChange, handleSkipPatternsChange, handleMaskPatternsChange) currently call saveSetting and updateScript on every keystroke; change them to only update local state (use setSamplingRate, setSkipPatterns, setMaskPatterns) on change, and move the saveSetting("...", value) + conditional updateScript() calls into separate onBlur handlers (e.g., saveSamplingRate, saveSkipPatterns, saveMaskPatterns) or implement a debounce that persists after a short idle; ensure the new onBlur/debounced functions call saveSetting and, if isInstalled, updateScript so framer.setCustomCode is not invoked on every character.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/components.ts`:
- Around line 88-114: TrackView's wrapper uses style {{ display: "contents" }},
which yields no box so IntersectionObserver.observe(ref.current) never fires;
fix by giving the observed element a real layout box or observing a real child:
either remove display: "contents" and use a block-level wrapper (update the div
in TrackView) so ref.current has a bounding box, OR instead attach the
ref/observer to the first rendered child element (use
React.Children.toArray(props.children)[0] and cloneElement to forward the ref or
find the child's DOM node) and observe that child; ensure fired/ref/observer
logic in TrackView remains the same and keep
props.threshold/props.eventName/props.properties in the effect dependencies.
In `@src/lib/script.ts`:
- Around line 83-107: The includeSampling condition currently excludes 1.0, so a
user-entered samplingNumber of exactly 1 is saved but never emitted; update the
logic around samplingNumber/includeSampling/samplingAttr so values in the full
[0, 1] range are accepted (i.e., use samplingNumber <= 1 instead of < 1) and
ensure samplingAttr is set when samplingNumber === 1.0 (so
attr("data-sampling-rate", String(samplingNumber)) is emitted for 1.0 as well),
referencing the variables samplingRaw, samplingNumber, includeSampling, and
samplingAttr when making the change.
---
Outside diff comments:
In `@src/App.tsx`:
- Around line 286-317: The three handlers (handleSamplingRateChange,
handleSkipPatternsChange, handleMaskPatternsChange) currently call saveSetting
and updateScript on every keystroke; change them to only update local state (use
setSamplingRate, setSkipPatterns, setMaskPatterns) on change, and move the
saveSetting("...", value) + conditional updateScript() calls into separate
onBlur handlers (e.g., saveSamplingRate, saveSkipPatterns, saveMaskPatterns) or
implement a debounce that persists after a short idle; ensure the new
onBlur/debounced functions call saveSetting and, if isInstalled, updateScript so
framer.setCustomCode is not invoked on every character.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ce42d22-c245-4a60-8da9-23aa09151c22
📒 Files selected for processing (4)
src/App.tsxsrc/hooks/use-component-file.tssrc/lib/components.tssrc/lib/script.ts
| export function TrackView(props) { | ||
| const ref = useRef(null) | ||
| const fired = useRef(false) | ||
| useEffect(() => { | ||
| const el = ref.current | ||
| if (!el) return | ||
| const observer = new IntersectionObserver( | ||
| ([entry]) => { | ||
| if (entry.isIntersecting && !fired.current) { | ||
| fired.current = true | ||
| fire(props.eventName, props.properties) | ||
| } | ||
| }, | ||
| { threshold: props.threshold ?? 0.5 } | ||
| ) | ||
| observer.observe(el) | ||
| return () => observer.disconnect() | ||
| }, [props.eventName, props.properties, props.threshold]) | ||
| return ( | ||
| <div | ||
| ref={ref} | ||
| style={{ width: "100%", height: "100%", display: "contents" }} | ||
| > | ||
| {props.children} | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does IntersectionObserver reliably report isIntersecting for an element with display: contents in Chrome, Safari, and Firefox?
💡 Result:
No, IntersectionObserver does not reliably report isIntersecting for an element with display: contents in Chrome, Safari, or Firefox. Elements styled with display: contents generate no layout box, so element.getBoundingClientRect returns a zero-area rectangle (width: 0, height: 0). IntersectionObserver uses this boundingClientRect to compute intersections per the specification; a zero-area targetRect results in intersectionRatio of 0 or 1 only if edge-adjacent, but typically no meaningful intersection callbacks occur, and isIntersecting remains false even if children are visible. This is by design, consistent across browsers, with no browser-specific bugs found in recent sources (up to 2026). Workarounds include observing child elements instead.
Citations:
- 1: https://stackoverflow.com/questions/72741599/intersectionobserver-does-not-intercept-elements-with-display-contents
- 2: UseInView doesn't work with display: 'contents'. I would like to observe child element. thebuilder/react-intersection-observer#421
- 3: https://stackoverflow.com/questions/75454061/getboundingclientrect-from-a-div-with-as-style-display-contents
- 4: [cssom-view]
getClientRects()should return a flattened list of child rects fordisplay: contentsw3c/csswg-drafts#12040 - 5: Incorrect positioning when reference has
display: contentsfloating-ui/floating-ui#2403 - 6: https://w3c.github.io/IntersectionObserver
- 7: https://developer.mozilla.org/en-US/docs/Web/API/element/getBoundingClientRect
display: contents on the observed element breaks IntersectionObserver.
The wrapper that carries ref for the observer uses style={{ width: "100%", height: "100%", display: "contents" }}. An element with display: contents generates no principal box—it produces a zero-area bounding rectangle—so IntersectionObserver.observe(el) on such an element will not fire, regardless of whether the rendered children are in view. This is spec-compliant behavior across all browsers (Chrome, Safari, Firefox) and directly defeats the purpose of TrackView.
Two reasonable options:
- Remove
display: contentsfrom the wrapper and let it render withdisplay: block, or - Observe the first rendered child element instead of the contents-only wrapper.
♻️ Minimal patch — give TrackView its own box
- return (
- <div
- ref={ref}
- style={{ width: "100%", height: "100%", display: "contents" }}
- >
- {props.children}
- </div>
- )
+ return (
+ <div
+ ref={ref}
+ style={{ width: "100%", height: "100%" }}
+ >
+ {props.children}
+ </div>
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function TrackView(props) { | |
| const ref = useRef(null) | |
| const fired = useRef(false) | |
| useEffect(() => { | |
| const el = ref.current | |
| if (!el) return | |
| const observer = new IntersectionObserver( | |
| ([entry]) => { | |
| if (entry.isIntersecting && !fired.current) { | |
| fired.current = true | |
| fire(props.eventName, props.properties) | |
| } | |
| }, | |
| { threshold: props.threshold ?? 0.5 } | |
| ) | |
| observer.observe(el) | |
| return () => observer.disconnect() | |
| }, [props.eventName, props.properties, props.threshold]) | |
| return ( | |
| <div | |
| ref={ref} | |
| style={{ width: "100%", height: "100%", display: "contents" }} | |
| > | |
| {props.children} | |
| </div> | |
| ) | |
| } | |
| export function TrackView(props) { | |
| const ref = useRef(null) | |
| const fired = useRef(false) | |
| useEffect(() => { | |
| const el = ref.current | |
| if (!el) return | |
| const observer = new IntersectionObserver( | |
| ([entry]) => { | |
| if (entry.isIntersecting && !fired.current) { | |
| fired.current = true | |
| fire(props.eventName, props.properties) | |
| } | |
| }, | |
| { threshold: props.threshold ?? 0.5 } | |
| ) | |
| observer.observe(el) | |
| return () => observer.disconnect() | |
| }, [props.eventName, props.properties, props.threshold]) | |
| return ( | |
| <div | |
| ref={ref} | |
| style={{ width: "100%", height: "100%" }} | |
| > | |
| {props.children} | |
| </div> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components.ts` around lines 88 - 114, TrackView's wrapper uses style
{{ display: "contents" }}, which yields no box so
IntersectionObserver.observe(ref.current) never fires; fix by giving the
observed element a real layout box or observing a real child: either remove
display: "contents" and use a block-level wrapper (update the div in TrackView)
so ref.current has a bounding box, OR instead attach the ref/observer to the
first rendered child element (use React.Children.toArray(props.children)[0] and
cloneElement to forward the ref or find the child's DOM node) and observe that
child; ensure fired/ref/observer logic in TrackView remains the same and keep
props.threshold/props.eventName/props.properties in the effect dependencies.
| const samplingRaw = await framer.getPluginData( | ||
| `${SETTING_PREFIX}samplingRate` | ||
| ); | ||
| const samplingNumber = samplingRaw ? Number(samplingRaw) : Number.NaN; | ||
| const includeSampling = | ||
| Number.isFinite(samplingNumber) && | ||
| samplingNumber >= 0 && | ||
| samplingNumber < 1; | ||
|
|
||
| const skipPatterns = await framer.getPluginData( | ||
| `${SETTING_PREFIX}skipPatterns` | ||
| ); | ||
| const maskPatterns = await framer.getPluginData( | ||
| `${SETTING_PREFIX}maskPatterns` | ||
| ); | ||
|
|
||
| const apiUrlAttr = | ||
| customApiUrl && isValidHttpUrl(customApiUrl) | ||
| ? attr("data-api-url", customApiUrl) | ||
| : ""; | ||
| const disabledAttr = | ||
| isDisabled === "true" ? `\n data-disabled="true"` : ""; | ||
| const samplingAttr = includeSampling | ||
| ? attr("data-sampling-rate", String(samplingNumber)) | ||
| : ""; |
There was a problem hiding this comment.
Sampling-rate upper bound excludes 1.0, but the UI accepts it.
includeSampling requires samplingNumber < 1, so a user entering exactly 1.0 (which the <input type="number" max="1"> at src/App.tsx lines 411-421 accepts and which is the natural "track 100%" value) will silently result in no data-sampling-rate attribute. That happens to be equivalent to the SDK default, but the UX is surprising — the value is saved yet never emitted.
Either (a) accept <= 1 and emit, letting the SDK ignore a 1.0, or (b) clamp/warn in the UI so 1.0 can't be entered as a "sampling" value. Option (a) is the least surprising:
♻️ Proposed tweak
- const includeSampling =
- Number.isFinite(samplingNumber) &&
- samplingNumber >= 0 &&
- samplingNumber < 1;
+ const includeSampling =
+ Number.isFinite(samplingNumber) &&
+ samplingNumber >= 0 &&
+ samplingNumber <= 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/script.ts` around lines 83 - 107, The includeSampling condition
currently excludes 1.0, so a user-entered samplingNumber of exactly 1 is saved
but never emitted; update the logic around
samplingNumber/includeSampling/samplingAttr so values in the full [0, 1] range
are accepted (i.e., use samplingNumber <= 1 instead of < 1) and ensure
samplingAttr is set when samplingNumber === 1.0 (so attr("data-sampling-rate",
String(samplingNumber)) is emitted for 1.0 as well), referencing the variables
samplingRaw, samplingNumber, includeSampling, and samplingAttr when making the
change.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Summary
TrackClick,TrackSubmit,TrackView) generated intoDatabuddy.tsx. Each has property controls so designers configure the event name and properties from Framer's right sidebar.<script>with current SDK options:data-debug,data-disabled,data-sampling-rate,data-skip-patterns,data-mask-patterns,data-use-pixel,data-ignore-bot-detection,data-api-url. Drops removedscrollDepthoption.#0099ffto the new Databuddy purple.setPluginDatanow respectsframer.isAllowedTo("setPluginData")so view-only collaborators don't hit silent failures.Test plan
Databuddy.tsxappears under Code Files, success toast says "3 components ready", card swaps to show "Find them in the Assets panel" + Remove.TrackClickonto a button on the canvas — right sidebar shows Event / Properties inputs; default event name isbutton_clicked.TrackSubmitonto a form, fire a submit, confirm the network tab shows the configured event.TrackViewover a section, scroll into view, confirm the event fires once at the configured threshold.data-disabled="true"appears in the injected script; toggle Resume to clear it.data-*attributes appear and the debug script URL switches.data-api-urlappears on the script.Summary by CodeRabbit
Release Notes
New Features
Style