✨ Add option to enable custom branding#2321
Merged
Conversation
Contributor
WalkthroughAdds a custom branding feature: new UI to toggle and save a space primary color, TRPC changes to accept/clear primaryColor and update showBranding with PostHog groupIdentify, locale strings, paywall gating for free users, a toast-progress hook, and conditional application of branding CSS vars. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as CustomBrandingSection
participant Billing as useBilling
participant Paywall as PayWallDialog
participant TRPC as TRPC (spaces)
participant DB as Database
participant PostHog
participant Cache as Query Cache
User->>UI: Toggle "Enable Custom Branding"
UI->>Billing: check isFree
alt Free user
Billing-->>UI: isFree = true
UI->>PostHog: capture event (attempt_enable_branding)
UI->>Paywall: show paywall
Paywall-->>User: display Pro benefits (includes custom branding)
else Paid user
Billing-->>UI: isFree = false
UI->>TRPC: mutate spaces.updateShowBranding(newValue)
TRPC->>DB: update showBranding
DB-->>TRPC: confirm
TRPC->>PostHog: groupIdentify(custom_branding_enabled)
TRPC->>Cache: update spaces.getCurrent (optimistic)
Cache-->>UI: reflect new state
UI->>User: toast "Branding saved"
end
User->>UI: Pick color & click Save
UI->>TRPC: mutate spaces.update({ primaryColor })
TRPC->>DB: update primaryColor (nullable)
DB-->>TRPC: confirm
TRPC->>Cache: update spaces.getCurrent
Cache-->>UI: reflect new color
UI->>User: toast "Branding saved"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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}
Contributor
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)
apps/web/src/trpc/routers/spaces.ts (1)
269-299:⚠️ Potential issue | 🟠 MajorAvoid clobbering
nameduring branding-only updates.Because
nameis required and always written (Line 271 and Line 295), a color-only save from stale client data can overwrite a newer space rename.💡 Suggested fix
update: spaceProcedure .input( - z.object({ - name: z.string().min(1).max(100), + z + .object({ + name: z.string().min(1).max(100).optional(), primaryColor: z .string() .regex(/^#[0-9a-fA-F]{6}$/, "Invalid hex color") .nullable() .optional(), - }), + }) + .refine( + (data) => data.name !== undefined || data.primaryColor !== undefined, + "At least one field must be provided", + ), ) .mutation(async ({ ctx, input }) => { @@ await prisma.space.update({ where: { id: ctx.space.id }, data: { - name: input.name, + ...(input.name !== undefined && { name: input.name }), ...(input.primaryColor !== undefined && { primaryColor: input.primaryColor, }), }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/trpc/routers/spaces.ts` around lines 269 - 299, The mutation currently requires and always writes name which causes color-only updates to clobber newer names; make the name field optional in the Zod input (change z.string().min(1).max(100) to optional()) and mirror the primaryColor pattern in the prisma.space.update call so name is only included in the data object when input.name !== undefined; update the input schema and the update payload in the mutation that uses defineAbilityForMember / memberAbility and prisma.space.update accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx:
- Around line 54-60: The free-tier guard in handleToggle currently blocks all
toggles; change it so it only blocks attempts to turn branding off. Update the
condition in handleToggle (referencing isFree, newChecked, and showPayWall) to
call showPayWall() and return only when isFree && newChecked === false,
otherwise proceed to call toastProgress with updateShowBranding.mutateAsync({
showBranding: newChecked }).
- Around line 62-66: The handleSave flow calls updateSpace.mutateAsync but
doesn't refresh the cached space data, so UI consumers of spaces.getCurrent
won't see the new primaryColor; after the mutation completes in handleSave,
trigger a refresh/invalidation of the spaces.getCurrent cache (via your
trpc/react-query utils or equivalent) so that spaces.getCurrent returns the
updated space and theming updates immediately.
---
Outside diff comments:
In `@apps/web/src/trpc/routers/spaces.ts`:
- Around line 269-299: The mutation currently requires and always writes name
which causes color-only updates to clobber newer names; make the name field
optional in the Zod input (change z.string().min(1).max(100) to optional()) and
mirror the primaryColor pattern in the prisma.space.update call so name is only
included in the data object when input.name !== undefined; update the input
schema and the update payload in the mutation that uses defineAbilityForMember /
memberAbility and prisma.space.update accordingly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fdb432f5-b895-4463-8a53-9b6d27d1d1b9
📒 Files selected for processing (9)
apps/web/public/locales/en/app.jsonapps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsxapps/web/src/app/[locale]/(space)/settings/general/components/space-branding-form.tsxapps/web/src/app/[locale]/(space)/settings/general/components/space-settings-form.tsxapps/web/src/app/[locale]/(space)/settings/general/page-client.tsxapps/web/src/features/billing/components/pay-wall-dialog.tsxapps/web/src/features/space/client.tsxapps/web/src/lib/use-toast-progress.tsapps/web/src/trpc/routers/spaces.ts
Comment on lines
+54
to
+60
| const handleToggle = (newChecked: boolean) => { | ||
| if (isFree) { | ||
| showPayWall(); | ||
| return; | ||
| } | ||
| toastProgress(updateShowBranding.mutateAsync({ showBranding: newChecked })); | ||
| }; |
Contributor
There was a problem hiding this comment.
Free-tier guard currently blocks turning branding off.
Line 55 applies the paywall guard regardless of target state, so free users with showBranding=true cannot disable it.
💡 Suggested fix
const handleToggle = (newChecked: boolean) => {
- if (isFree) {
+ if (newChecked && isFree) {
showPayWall();
return;
}
toastProgress(updateShowBranding.mutateAsync({ showBranding: newChecked }));
};📝 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.
Suggested change
| const handleToggle = (newChecked: boolean) => { | |
| if (isFree) { | |
| showPayWall(); | |
| return; | |
| } | |
| toastProgress(updateShowBranding.mutateAsync({ showBranding: newChecked })); | |
| }; | |
| const handleToggle = (newChecked: boolean) => { | |
| if (newChecked && isFree) { | |
| showPayWall(); | |
| return; | |
| } | |
| toastProgress(updateShowBranding.mutateAsync({ showBranding: newChecked })); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx
around lines 54 - 60, The free-tier guard in handleToggle currently blocks all
toggles; change it so it only blocks attempts to turn branding off. Update the
condition in handleToggle (referencing isFree, newChecked, and showPayWall) to
call showPayWall() and return only when isFree && newChecked === false,
otherwise proceed to call toastProgress with updateShowBranding.mutateAsync({
showBranding: newChecked }).
Owner
Author
There was a problem hiding this comment.
When a space gets downgraded we disable show branding so this shouldn't happen
Contributor
There was a problem hiding this comment.
@lukevella, that makes sense — since downgrading automatically disables showBranding, a free-tier space can never have it enabled in the first place, so the guard is correctly placed. Thanks for the clarification!
✏️ Learnings added
Learnt from: lukevella
Repo: lukevella/rallly PR: 2321
File: apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx:54-60
Timestamp: 2026-04-09T10:38:54.240Z
Learning: In the Rallly codebase (`apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx`), when a space is downgraded from a paid tier to free, `showBranding` is automatically set to `false`. Therefore, a free-tier space can never have `showBranding=true`, and the paywall guard in `handleToggle` (blocking all toggles for `isFree` users) is intentional and correct — free users will never need to turn branding off.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+62
to
+66
| const handleSave = async () => { | ||
| const value = hexColor === DEFAULT_PRIMARY_COLOR ? null : hexColor; | ||
| toastProgress( | ||
| updateSpace.mutateAsync({ name: space.name, primaryColor: value }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Refresh spaces.getCurrent after saving color.
The save mutation doesn’t update or invalidate the space cache, so branding changes may not apply immediately in UI that reads spaces.getCurrent (e.g., theming in apps/web/src/features/space/client.tsx).
💡 Suggested fix
const handleSave = async () => {
const value = hexColor === DEFAULT_PRIMARY_COLOR ? null : hexColor;
toastProgress(
- updateSpace.mutateAsync({ name: space.name, primaryColor: value }),
+ updateSpace
+ .mutateAsync({ name: space.name, primaryColor: value })
+ .then(() => utils.spaces.getCurrent.invalidate()),
);
};📝 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.
Suggested change
| const handleSave = async () => { | |
| const value = hexColor === DEFAULT_PRIMARY_COLOR ? null : hexColor; | |
| toastProgress( | |
| updateSpace.mutateAsync({ name: space.name, primaryColor: value }), | |
| ); | |
| const handleSave = async () => { | |
| const value = hexColor === DEFAULT_PRIMARY_COLOR ? null : hexColor; | |
| toastProgress( | |
| updateSpace | |
| .mutateAsync({ name: space.name, primaryColor: value }) | |
| .then(() => utils.spaces.getCurrent.invalidate()), | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx
around lines 62 - 66, The handleSave flow calls updateSpace.mutateAsync but
doesn't refresh the cached space data, so UI consumers of spaces.getCurrent
won't see the new primaryColor; after the mutation completes in handleSave,
trigger a refresh/invalidation of the spaces.getCurrent cache (via your
trpc/react-query utils or equivalent) so that spaces.getCurrent returns the
updated space and theming updates immediately.
Owner
Author
There was a problem hiding this comment.
Any mutation invalidates all queries
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for global mutation invalidation configuration in TRPC or React Query setup
rg -n "invalidate" apps/web/src/trpc/ -A3 -B3
rg -n "onSuccess|onSettled|onMutate" apps/web/src/trpc/ -A3 -B3 | head -80
# Also check for queryClient or ReactQueryProvider config
rg -rn "invalidateQueries\|onSuccess.*invalidate" apps/web/src/ -A3 -B3 | head -60
Length of output: 787
@lukevella, confirmed — apps/web/src/trpc/client.ts has a global override that calls opts.queryClient.invalidateQueries() (with no filter) in the onSuccess of every mutation, so spaces.getCurrent will already be refreshed automatically. My suggestion was unnecessary. Apologies for the noise!
✏️ Learnings added
Learnt from: lukevella
Repo: lukevella/rallly PR: 2321
File: apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx:62-66
Timestamp: 2026-04-09T10:39:30.660Z
Learning: In the Rallly codebase (`apps/web/src/trpc/client.ts`), the TRPC client is configured with a global `useMutation` override that calls `opts.queryClient.invalidateQueries()` (no arguments) inside `onSuccess`. This means every tRPC mutation automatically invalidates all React Query caches, so explicit per-mutation cache invalidation is never needed.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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 `@apps/web/src/trpc/routers/spaces.ts`:
- Around line 269-278: The input schema currently requires name and always
writes it back, causing color-only branding saves to overwrite newer names;
update the z.object in the mutation (the .input schema defining name and
primaryColor) to make name optional (and/or nullable) and then change the update
logic (the handler that calls prisma/ctx update — e.g., the space update
mutation around the code that writes name on save) to only include name in the
update payload when it is present in the input. Ensure primaryColor can still be
saved independently and that the update call merges fields conditionally so a
missing name won’t overwrite the DB.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e14f9498-0d9e-4fc9-9896-13f5ef342851
📒 Files selected for processing (3)
apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsxapps/web/src/features/space/client.tsxapps/web/src/trpc/routers/spaces.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx (2)
99-104: Usecn()for conditional class composition.Prefer
cn()over ternary class strings for consistency with the design-system utility pattern.Suggested change
+import { cn } from "@rallly/ui"; @@ - <div - className={ - !space.showBranding || disabled - ? "pointer-events-none opacity-50" - : undefined - } - > + <div + className={cn( + !space.showBranding || disabled && "pointer-events-none opacity-50", + )} + >As per coding guidelines, "Use
cn()from@rallly/uito compose classes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx around lines 99 - 104, Replace the ternary className in the CustomBrandingSection JSX with the design-system cn() utility: import cn from "@rallly/ui" (or ensure it's already imported) and change the div's className to use cn() to conditionally apply "pointer-events-none opacity-50" when !space.showBranding || disabled; keep undefined/no class when false (e.g., className={cn({ "pointer-events-none opacity-50": !space.showBranding || disabled })} or cn(!space.showBranding || disabled && "pointer-events-none opacity-50")). Ensure you update the component named CustomBrandingSection accordingly.
58-58: Align PostHog event action naming to present tense.Use a present-tense action in the event key to match the repository convention.
Suggested change
- posthog?.capture("branding_settings:custom_branding_paywall_shown"); + posthog?.capture("branding_settings:custom_branding_paywall_show");As per coding guidelines, "Use the
category:object_actionpattern for PostHog event naming with lowercase, snake_case, and present-tense verbs..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx at line 58, The PostHog event name uses past tense ("branding_settings:custom_branding_paywall_shown"); update the string in the posthog?.capture call to use present-tense and follow the category:object_action pattern and snake_case (for example "branding_settings:custom_branding_paywall_show" or "branding_settings:custom_branding_paywall_view") so replace the existing event key in the posthog?.capture invocation accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx:
- Around line 99-104: Replace the ternary className in the CustomBrandingSection
JSX with the design-system cn() utility: import cn from "@rallly/ui" (or ensure
it's already imported) and change the div's className to use cn() to
conditionally apply "pointer-events-none opacity-50" when !space.showBranding ||
disabled; keep undefined/no class when false (e.g., className={cn({
"pointer-events-none opacity-50": !space.showBranding || disabled })} or
cn(!space.showBranding || disabled && "pointer-events-none opacity-50")). Ensure
you update the component named CustomBrandingSection accordingly.
- Line 58: The PostHog event name uses past tense
("branding_settings:custom_branding_paywall_shown"); update the string in the
posthog?.capture call to use present-tense and follow the category:object_action
pattern and snake_case (for example
"branding_settings:custom_branding_paywall_show" or
"branding_settings:custom_branding_paywall_view") so replace the existing event
key in the posthog?.capture invocation accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a035780b-5d06-4cc7-9aa4-8ca556f87eff
📒 Files selected for processing (2)
apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsxapps/web/src/trpc/routers/spaces.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/trpc/routers/spaces.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx (1)
99-104: Usecn()for conditional Tailwind class composition.This conditional
classNameshould usecn()instead of a ternary string for consistency with project conventions.♻️ Proposed refactor
+import { cn } from "@rallly/ui"; ... - <div - className={ - !space.showBranding || disabled - ? "pointer-events-none opacity-50" - : undefined - } - > + <div + className={cn( + (!space.showBranding || disabled) && + "pointer-events-none opacity-50", + )} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx around lines 99 - 104, The div's conditional className currently uses a ternary string; replace it with the project's cn() helper to compose Tailwind classes consistently: change the div's className to something like cn({ "pointer-events-none opacity-50": !space.showBranding || disabled }) (or include other static classes as first args) and ensure the cn helper is imported (the project's cn utility) so the conditional class is applied via cn instead of the ternary; update the JSX in the custom-branding-section component where the div uses space.showBranding and disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx:
- Around line 99-104: The div's conditional className currently uses a ternary
string; replace it with the project's cn() helper to compose Tailwind classes
consistently: change the div's className to something like cn({
"pointer-events-none opacity-50": !space.showBranding || disabled }) (or include
other static classes as first args) and ensure the cn helper is imported (the
project's cn utility) so the conditional class is applied via cn instead of the
ternary; update the JSX in the custom-branding-section component where the div
uses space.showBranding and disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2b136dbd-2096-4c83-b5e1-0edcf9456937
📒 Files selected for processing (1)
apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx
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 by CodeRabbit