fix(platform): improve branding settings UX and dark mode support#497
Merged
Conversation
- Fix color picker dirty state by normalizing hex case comparison - Support 8-character hex colors with alpha channel (#RRGGBBAA) - Track image uploads via form state for proper dirty detection - Support image deletion with null storage ID handling - Add dark mode support to branding preview with semantic tokens - Show app name and favicon in browser chrome preview - Use accent color for tab nav indicator, brand color for logo only - Reorder settings nav: Branding after API keys, Account last - Accept .ico files for favicon uploads - Add Figma capture script for local development
Greptile SummaryThis PR enhances the branding settings UX with better form state management and dark mode support. Key improvements include:
Issue found: Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| services/platform/app/features/settings/branding/components/color-picker-input.tsx | Added hex normalization to prevent false dirty state and support for 8-char hex codes with alpha channel |
| services/platform/app/features/settings/branding/components/branding-form.tsx | Refactored to use React Hook Form state instead of refs; preview URL refs declared but never updated |
| services/platform/app/features/settings/branding/components/branding-preview.tsx | Added browser chrome with app name and favicon; replaced hardcoded colors with semantic tokens for dark mode |
| services/platform/convex/branding/mutations.ts | Added null support for storage IDs to enable deletion; properly converts null to undefined for database |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User uploads image] --> B[ImageUploadField]
B --> C[Create object URL for preview]
B --> D[Upload to Convex storage]
D --> E[Get storageId]
E --> F[Call onUpload with storageId]
F --> G[BrandingForm: setValue with storageId]
G --> H{User clicks Save}
H --> I[toStorageId converts string to Id]
I --> J[upsertBranding mutation]
J --> K{Storage ID is null?}
K -->|Yes| L[Convert null to undefined]
K -->|No| M[Delete old storage file if exists]
L --> N[Update database]
M --> N
N --> O[Form reset with new values]
P[User removes image] --> Q[ImageUploadField: onRemove]
Q --> R[BrandingForm: setValue with empty string]
R --> H
Last reviewed commit: 7795f3d
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughThis pull request enhances the branding settings feature by adding favicon support across the component stack. Changes include updating BrandingPreview and BrowserChrome components to accept and display favicon URLs, extending the color picker to support 8-character hex codes with alpha channel transparency, and refactoring form state management to use useState instead of useRef. Backend updates include Convex schema and mutation extensions to handle nullable favicon storage IDs (light and dark variants) with revised deletion logic. Tests are updated to reflect new functionality with accentColor assertions and placeholder bar assertions. A development-only Figma MCP script is conditionally injected in index.html, and settings navigation tab ordering is adjusted. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/index.html`:
- Around line 37-47: The script auto-inject currently checks location.hostname
and uses var; change it to require an explicit opt-in via a query parameter
(e.g., check location.search for a token like "enableFigmaCapture=true" or a
short flag) before creating/loading the third‑party capture script, and replace
the use of `var s` with `const s`; in other words, update the conditional that
checks `location.hostname === 'localhost' || location.hostname === '127.0.0.1'`
to also require `location.search.includes('enableFigmaCapture')` (or similar
flag), then create `const s = document.createElement('script')`, set s.src to
'https://mcp.figma.com/mcp/html-to-design/capture.js', set s.async = true, and
append only when the opt‑in flag is present.
Comment on lines
+37
to
+47
| <script> | ||
| if ( | ||
| location.hostname === 'localhost' || | ||
| location.hostname === '127.0.0.1' | ||
| ) { | ||
| var s = document.createElement('script'); | ||
| s.src = 'https://mcp.figma.com/mcp/html-to-design/capture.js'; | ||
| s.async = true; | ||
| document.head.appendChild(s); | ||
| } | ||
| </script> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if there is an existing dev/prod flag to use for gating (e.g., window.__ENV__).
rg -n "__ENV__" -S
rg -n "NODE_ENV|MODE|development|prod" -S
# Check for any CSP configuration that might affect inline scripts.
rg -n "Content-Security-Policy|CSP" -SRepository: tale-project/tale
Length of output: 50374
🏁 Script executed:
# Check the env utility to understand how window.__ENV__ is used
head -50 services/platform/lib/env.ts
# Check the inject-env plugin to see what gets exposed
head -60 services/platform/vite-plugins/inject-env.ts
# Check full context around the Figma script in index.html
head -60 services/platform/index.html
# Look for any existing isDev or development flags
rg -n "isDev|isDevelopment|development.*true" services/platform/lib/env.ts -A 2
rg -n "MODE.*development|ENV.*dev" services/platform/vite-plugins/inject-env.ts -A 2Repository: tale-project/tale
Length of output: 4061
Gate the Figma capture script behind explicit opt‑in on localhost.
Auto-injecting a third‑party capture script whenever the hostname is localhost or 127.0.0.1 can unintentionally exfiltrate sensitive local data during development (e.g., when testing with real data). Add an explicit opt‑in via query parameter so the script only loads when a developer intentionally enables it.
Also replace var with const for consistency.
🔧 Proposed fix (query param gate + const)
- if (
- location.hostname === 'localhost' ||
- location.hostname === '127.0.0.1'
- ) {
- var s = document.createElement('script');
- s.src = 'https://mcp.figma.com/mcp/html-to-design/capture.js';
- s.async = true;
- document.head.appendChild(s);
- }
+ const enableFigmaCapture =
+ (location.hostname === 'localhost' ||
+ location.hostname === '127.0.0.1') &&
+ new URLSearchParams(location.search).has('figmaCapture');
+
+ if (enableFigmaCapture) {
+ const s = document.createElement('script');
+ s.src = 'https://mcp.figma.com/mcp/html-to-design/capture.js';
+ s.async = true;
+ document.head.appendChild(s);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/index.html` around lines 37 - 47, The script auto-inject
currently checks location.hostname and uses var; change it to require an
explicit opt-in via a query parameter (e.g., check location.search for a token
like "enableFigmaCapture=true" or a short flag) before creating/loading the
third‑party capture script, and replace the use of `var s` with `const s`; in
other words, update the conditional that checks `location.hostname ===
'localhost' || location.hostname === '127.0.0.1'` to also require
`location.search.includes('enableFigmaCapture')` (or similar flag), then create
`const s = document.createElement('script')`, set s.src to
'https://mcp.figma.com/mcp/html-to-design/capture.js', set s.async = true, and
append only when the opt‑in flag is present.
8 tasks
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#RRGGBBAA)nullstorage IDs in the Convex mutationbg-background,text-foreground,border-border, etc.).icofiles for favicon uploadsTest plan
#3B34FE1A) save and display correctly.icofile as faviconSummary by CodeRabbit
Release Notes
New Features
Style