-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add privacy line #1890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add privacy line #1890
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughUpdated the access form’s footer when useCustomAccessForm is false: adjusted layout to a vertical stack, changed the Papermark link URL and styling, added a Privacy Policy line with an external-link icon, and imported the required icon. Minor punctuation change. Changes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (3)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/view/access-form/email-section.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
components/view/access-form/email-section.tsx (2)
4-4: LGTM!The import is correctly added and the icon is used appropriately in the Privacy Policy link.
152-152: Ensure NEXT_PUBLIC_MARKETING_URL is defined in all deployment environments
It’s only declared in.env.example; confirm it’s set in your actual.env.local/.env.productionfiles or in your hosting platform’s environment settings to avoid broken links.
| <p className="text-sm text-gray-500"> | ||
| {useCustomAccessForm | ||
| ? "This data will be shared with the content provider." | ||
| : "This data will be shared with the sender."} | ||
| ? "This data will be shared with the content provider. " | ||
| : "This data will be shared with the sender. "} | ||
| Learn more about how we use and protect your data in our{" "} | ||
| <a | ||
| href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="flex items-center text-gray-500 hover:text-gray-600" | ||
| > | ||
| <span>Privacy Policy</span> | ||
| <ArrowUpRightIcon className="h-4 w-4 text-gray-500" /> | ||
| </a> | ||
| . | ||
| </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the link layout to maintain inline text flow.
The flex items-center class on the anchor tag will break the paragraph's text flow, causing the link to become a block-level element instead of staying inline with the surrounding text.
Apply this diff to fix the layout:
- <a
- href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`}
- target="_blank"
- rel="noopener noreferrer"
- className="flex items-center text-gray-500 hover:text-gray-600"
- >
- <span>Privacy Policy</span>
- <ArrowUpRightIcon className="h-4 w-4 text-gray-500" />
- </a>
+ <a
+ href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`}
+ target="_blank"
+ rel="noopener noreferrer"
+ className="inline-flex items-center gap-0.5 text-gray-500 hover:text-gray-600"
+ >
+ Privacy Policy
+ <ArrowUpRightIcon className="h-3 w-3" aria-hidden="true" />
+ </a>Changes:
- Use
inline-flexinstead offlexto maintain inline flow - Add
gap-0.5for consistent spacing between text and icon - Remove unnecessary
<span>wrapper - Add
aria-hidden="true"to the icon since it's decorative - Slightly reduce icon size to
h-3 w-3for better proportion with inline text
📝 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.
| <p className="text-sm text-gray-500"> | |
| {useCustomAccessForm | |
| ? "This data will be shared with the content provider." | |
| : "This data will be shared with the sender."} | |
| ? "This data will be shared with the content provider. " | |
| : "This data will be shared with the sender. "} | |
| Learn more about how we use and protect your data in our{" "} | |
| <a | |
| href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="flex items-center text-gray-500 hover:text-gray-600" | |
| > | |
| <span>Privacy Policy</span> | |
| <ArrowUpRightIcon className="h-4 w-4 text-gray-500" /> | |
| </a> | |
| . | |
| </p> | |
| <p className="text-sm text-gray-500"> | |
| {useCustomAccessForm | |
| ? "This data will be shared with the content provider. " | |
| : "This data will be shared with the sender. "} | |
| Learn more about how we use and protect your data in our{" "} | |
| <a | |
| href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="inline-flex items-center gap-0.5 text-gray-500 hover:text-gray-600" | |
| > | |
| Privacy Policy | |
| <ArrowUpRightIcon className="h-3 w-3" aria-hidden="true" /> | |
| </a> | |
| . | |
| </p> |
🤖 Prompt for AI Agents
components/view/access-form/email-section.tsx lines 146-161: the anchor uses
"flex items-center" which breaks inline paragraph flow; change it to
"inline-flex items-center gap-0.5" to keep the link inline and add small
spacing, remove the unnecessary <span> wrapper around the link text so the text
and icon are direct children of the <a>, set the icon props to
aria-hidden="true" and reduce its classes to "h-3 w-3" for proper inline sizing.
| href={`${process.env.NEXT_PUBLIC_MARKETING_URL}/privacy`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="flex items-center text-gray-500 hover:text-gray-600" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| className="flex items-center text-gray-500 hover:text-gray-600" | |
| className="inline-flex items-center text-gray-500 hover:text-gray-600" |
The privacy policy link uses flex items-center styling which is inappropriate for inline text and inconsistent with other privacy policy links in the codebase.
View Details
Analysis
Privacy policy link uses block-level flex instead of inline-flex in paragraph text
What fails: Privacy policy link in EmailSection component uses flex items-center which creates a block-level flex container, disrupting text flow within the paragraph
How to reproduce:
// In components/view/access-form/email-section.tsx line 155
<a className="flex items-center text-gray-500 hover:text-gray-600">
<span>Privacy Policy</span>
<ArrowUpRightIcon />
</a>Result: The link breaks normal inline text flow because flex creates a block-level container instead of inline-level
Expected: Should use inline-flex items-center for proper inline behavior within paragraph text, as confirmed by CSS flexbox documentation
Inconsistency: Other privacy policy links in the codebase use underline patterns without flex layout for inline text
Summary by CodeRabbit
New Features
Style