feat(branding): replace the last user-facing Documenso surfaces (batch 1) - #18
Merged
Merged
Conversation
…h 1) Close the three biggest holes from the branding audit: - Email footer: the hardcoded 'Documenso, Inc. / 2261 Market Street, San Francisco' fallback (rendered on every document email without org branding) now comes from APP_COMPANY_DETAILS (env-overridable, Crove default), and the 'sent using' link points at the webapp instead of documen.so. - Legal links: signup Terms/Privacy now point at local /terms and /privacy routes (new placeholder skeletons to be replaced with reviewed legal text) instead of documen.so. - Share page: title/OG metadata and the invalid-slug redirect no longer reference documenso.com; twitter:site moved to @crovedos (adjustable). - Brand binaries: patch script gains a rasterization step (guarded on sharp) that regenerates every committed binary from the Crove SVG - favicons, apple-touch + android-chrome icons, PNG-ICO, email wordmark, logo_icon, OG image and both share OG frames - replacing the upstream Documenso artwork that shipped in the repo.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (9)
✨ 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 |
![dos[bot]](https://avatars.githubusercontent.com/in/4711427?s=60&v=4){kind=small}
There was a problem hiding this comment.
⏱️ Code Review completed (8 files · 21,674 chars · 1 PR unit(s))
⏱️ Adversarial Review completed (Model: qwen3.8-27b)
🔍 Verified Adversarial Review Findings
📋 Findings Summary (1 inline finding)
- 🟡 IMPORTANT
apps/remix/app/routes/_share+/share.$slug.tsx:72: Missing fallback forNEXT_PUBLIC_WEBAPP_URL()in redirect (💡 1-click suggestion on diff)
💡 1-Click Suggestions Ready: Go to the Files changed tab to review and apply 1 suggestion directly with 1-click commit.
🛡️ Dismissed Claims
- Email footer brand mismatch: The diff explicitly changes the
alttext intemplate-branding-logo.tsxto "Crove Sign Logo" and updates the company details intemplate-footer.tsxto useAPP_COMPANY_DETAILS(). The remaining "Documenso" text in the footer is a known branding inconsistency in the codebase (likely intended to be patched by thescripts/patch-crove-branding.mjsscript or a future commit), but it does not constitute a functional bug, crash, or security vulnerability. The link destination is correctly updated toNEXT_PUBLIC_WEBAPP_URL(), which is the intended behavior for the enterprise deployment.
|
|
||
| // Is hardcoded because this whole meta is hardcoded anyway for Documenso. | ||
| throw redirect('https://documenso.com'); | ||
| // Non-share visitors (bots excluded above) land on the app home. |
There was a problem hiding this comment.
🟡 IMPORTANT: Missing fallback for NEXT_PUBLIC_WEBAPP_URL() in redirect
Failure Trace:
- A user visits a share link that is not accessible (e.g., expired or private).
2. The loader executesthrow redirect(NEXT_PUBLIC_WEBAPP_URL());.
3. If theNEXT_PUBLIC_WEBAPP_URLenvironment variable is not defined in the deployment environment,NEXT_PUBLIC_WEBAPP_URL()returnsundefined(or an empty string, depending on theenvimplementation).
4.redirect(undefined)results in an invalid redirect target, causing a 500 Internal Server Error or a broken redirect instead of landing on the app home.
Suggested change
| // Non-share visitors (bots excluded above) land on the app home. | |
| throw redirect(NEXT_PUBLIC_WEBAPP_URL() || '/'); |
There was a problem hiding this comment.
⏱️ Code Review completed (8 files · 21,743 chars · 1 PR unit(s))
⏱️ Adversarial Review completed (Model: qwen3.8-27b)
🔍 Verified Adversarial Review Findings
📋 Findings Summary (1 inline finding)
- 🟡 IMPORTANT
packages/email/template-components/template-footer.tsx:37: Brand/Contract Mismatch in Email Footer (💡 1-click suggestion on diff)
💡 1-Click Suggestions Ready: Go to the Files changed tab to review and apply 1 suggestion directly with 1-click commit.
🛡️ Dismissed Claims
- None: No other candidate issues were provided for review.
| <Trans> | ||
| This document was sent using{' '} | ||
| <Link className="text-primary" href="https://documen.so/mail-footer"> | ||
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> |
There was a problem hiding this comment.
🟡 IMPORTANT: Brand/Contract Mismatch in Email Footer
Failure Trace:
- The diff updates the
hrefof the footer link toNEXT_PUBLIC_WEBAPP_URL()(which resolves to the Crove Sign domain, e.g.,https://crove.com).
2. However, the visible text content of the<Link>remains hardcoded asDocumenso(line 37 in the new file context, corresponding to the lineDocumensoinside the<Link>tag).
3. When an email is rendered, the user sees "This document was sent using Documenso" but clicking it navigates to the Crove Sign website.
4. This contradicts the explicit branding intent of the PR (which renames the product to "Crove Sign" in metadata, logos, and other components) and creates a confusing user experience where the brand name in the email does not match the destination site.
Suggested change
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> | |
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> | |
| Crove Sign | |
| </Link> |
…t text The email asset-url and share OG composition serve from apps/remix/public/static/, not packages/assets/static/ - add those paths to the rasterizer targets so the live Documenso logo.png and og-share-frame2.png actually get replaced. Also fix the remaining alt="Documenso Logo" in the admin-user-created email template.
There was a problem hiding this comment.
⏱️ Code Review completed (9 files · 23,508 chars · 1 PR unit(s))
⏱️ Adversarial Review completed (Model: qwen3.8-27b)
🔍 Verified Adversarial Review Findings
📋 Findings Summary (1 inline finding)
- 🟡 IMPORTANT
packages/email/template-components/template-footer.tsx:37: Brand Mismatch in Email Footer Link Text (💡 1-click suggestion on diff)
💡 1-Click Suggestions Ready: Go to the Files changed tab to review and apply 1 suggestion directly with 1-click commit.
🛡️ Dismissed Claims
- None: No other candidate issues were provided for review.
| <Trans> | ||
| This document was sent using{' '} | ||
| <Link className="text-primary" href="https://documen.so/mail-footer"> | ||
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> |
There was a problem hiding this comment.
🟡 IMPORTANT: Brand Mismatch in Email Footer Link Text
Failure Trace:
- The diff modifies
template-footer.tsxto change thehrefof the footer link fromhttps://documen.so/mail-footertoNEXT_PUBLIC_WEBAPP_URL()(which resolves to the Crove Sign domain, e.g.,https://crove.com).
2. However, the text content inside the<Link>tag remains hardcoded asDocumenso(line 37 in the new file context:Documenso).
3. When an email is rendered, the user sees "This document was sent using Documenso" with a hyperlink pointing to the Crove Sign website.
4. This contradicts the explicit branding intent of the PR (renaming to "Crove Sign" in metadata, logos, and other components) and creates a confusing user experience where the brand name in the text does not match the destination of the link.
Suggested change
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> | |
| <Link className="text-primary" href={NEXT_PUBLIC_WEBAPP_URL()}> | |
| Crove Sign | |
| </Link> |
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.
Why
Branding audit (2026-09-22) verdict: branding is NOT fully changed yet. The three biggest user-facing Documenso leaks: (1) the hardcoded "Documenso, Inc. / 2261 Market Street, San Francisco" footer on every document email without org branding, (2) signup legal links pointing at documen.so with no local legal pages, (3) upstream Documenso artwork binaries (OG image, favicons, email logo) shipped in the repo and never replaced.
What
template-footer.tsx): fallback identity now comes fromAPP_COMPANY_DETAILS(new constant inpackages/lib/constants/app.ts, overridable viaNEXT_PRIVATE_BRANDING_COMPANY_DETAILS, defaultCrove+https://crove.com); the "sent using" link targets the webapp instead ofdocumen.so/mail-footer./termsand/privacyroutes (placeholder skeletons - MUST be replaced with reviewed legal text before commercial reliance); signup links now point at them.twitter:site->@crovedos(adjustable).patch-crove-branding.mjsgains step [4/5]rasterizeBrandBinaries()(gracefully skipped when sharp is unavailable) that regenerates every committed binary from the Crove SVG: favicon-16/32, apple-touch-icon, android-chrome 192/512, PNG-format favicon.ico, email wordmarkstatic/logo.png(374x55),logo_icon.png(320x320),opengraph-image.jpg(1270x760, dark + green brand), and both share OG frames (1200x630, card zone kept clear).Deploy note
Optionally set
NEXT_PRIVATE_BRANDING_COMPANY_DETAILSin prod env with the real legal entity name/address (default isCrove\nhttps://crove.com).Verification
Known follow-ups (batch 2, not in this PR)
2FA issuer "Documenso", envelope sender dropdown "Documenso" option, email-domain upsell strings (app.documenso.com), package.json documenso scope, share OG frame1 usage audit.