-
Notifications
You must be signed in to change notification settings - Fork 1k
UI: Added enable 'Secured by Papermark' in data rooms #1751
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
base: main
Are you sure you want to change the base?
UI: Added enable 'Secured by Papermark' in data rooms #1751
Conversation
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new boolean property, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant API
participant DB
User->>UI: Open Link/Dataroom Options
UI->>UI: Show SecuredByPapermarkSection toggle (if applicable)
User->>UI: Toggle "Secured by Papermark"
UI->>API: Save link with securedByPapermark=true/false
API->>DB: Update Link/LinkPreset.securedByPapermark
DB-->>API: Confirm update
API-->>UI: Respond with updated link data
User->>UI: View Dataroom/Document
UI->>API: Fetch link data (includes securedByPapermark)
API->>DB: Query Link.securedByPapermark
DB-->>API: Return link data
API-->>UI: Return link data
UI->>UI: If securedByPapermark, render SecuredByPapermark footer
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (2)
components/links/link-sheet/index.tsx (1)
81-81
: Simplify the boolean expression.The ternary operator with boolean literals is unnecessary and can be simplified for better readability.
Apply this diff to simplify the expression:
- securedByPapermark: linkType === LinkType.DATAROOM_LINK ? true : false, + securedByPapermark: linkType === LinkType.DATAROOM_LINK,prisma/migrations/20250711074827_add_secured_by_papermark/migration.sql (1)
1-9
: LGTM: Database schema changes are correctly implementedThe migration properly adds the
securedByPapermark
boolean column to bothLink
andLinkPreset
tables with appropriate default values.However, the dropped index
"Viewer_teamId_createdAt_idx"
appears unrelated to the new feature. Consider separating unrelated schema changes into different migrations for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
components/links/link-sheet/index.tsx
(3 hunks)components/links/link-sheet/link-options.tsx
(2 hunks)components/links/link-sheet/secured-by-papermark-section.tsx
(1 hunks)components/links/links-table.tsx
(1 hunks)components/view/dataroom/dataroom-document-view.tsx
(2 hunks)components/view/dataroom/dataroom-view.tsx
(1 hunks)components/view/secured-by-papermark.tsx
(1 hunks)components/view/viewer/dataroom-viewer.tsx
(5 hunks)components/welcome/containers/onboarding-dataroom-link-options.tsx
(2 hunks)pages/api/links/[id]/documents/[documentId].ts
(1 hunks)pages/api/links/[id]/index.ts
(2 hunks)pages/api/links/domains/[...domainSlug].ts
(1 hunks)pages/api/links/index.ts
(1 hunks)prisma/migrations/20250711074827_add_secured_by_papermark/migration.sql
(1 hunks)prisma/schema/link.prisma
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/welcome/containers/onboarding-dataroom-link-options.tsx (1)
components/links/link-sheet/secured-by-papermark-section.tsx (1)
SecuredByPapermarkSection
(8-51)
🪛 Biome (1.9.4)
components/links/link-sheet/index.tsx
[error] 81-81: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (23)
components/view/dataroom/dataroom-view.tsx (1)
291-291
: LGTM! Proper boolean coercion and consistent prop naming.The use of double negation (
!!
) ensures the prop is always a boolean, and the prop name matches the database field for consistency.pages/api/links/[id]/documents/[documentId].ts (1)
50-50
: LGTM! Correctly includes the field in API responses.The addition of
securedByPapermark: true
to the Prisma select ensures this field is available for frontend consumption.components/welcome/containers/onboarding-dataroom-link-options.tsx (2)
21-21
: LGTM! Correct import statement.The import follows the established pattern for other link sheet components.
90-94
: LGTM! Proper component integration.The
SecuredByPapermarkSection
is correctly placed in thealwaysShown
section with the appropriate props (data
,setData
,presets
) matching the component interface.components/view/dataroom/dataroom-document-view.tsx (2)
22-22
: LGTM! Correct import statement.The import follows the established pattern and naming conventions.
361-361
: LGTM! Proper conditional rendering.The conditional rendering ensures the
SecuredByPapermark
component only appears when the flag is enabled, and thelinkId
prop is correctly passed.pages/api/links/index.ts (1)
187-187
: LGTM! Correct property persistence.The
securedByPapermark
property is correctly included in the link creation data, following the established pattern for other boolean properties.pages/api/links/domains/[...domainSlug].ts (1)
56-56
: LGTM: Field addition is correctly implemented.The addition of
securedByPapermark
to the Prisma select clause is appropriate and consistent with the feature rollout across the API layer.components/links/links-table.tsx (1)
210-210
: LGTM: Proper null handling implementation.The use of the null coalescing operator (
??
) provides appropriate fallback behavior, ensuring thesecuredByPapermark
property is always set to a valid boolean value when editing links.prisma/schema/link.prisma (2)
45-45
: LGTM: Schema addition is well-designed.The
securedByPapermark
field is properly defined as an optional Boolean with a sensible default value offalse
, which is appropriate for this type of feature flag.
121-121
: LGTM: Consistent schema implementation.The addition of
securedByPapermark
to theLinkPreset
model maintains consistency with theLink
model and follows the same pattern with optional Boolean and default value.components/links/link-sheet/link-options.tsx (2)
37-37
: LGTM: Import statement is properly added.The import for
SecuredByPapermarkSection
is correctly placed and follows the established import organization pattern.
305-310
: LGTM: Conditional rendering is appropriately implemented.The conditional rendering for
DATAROOM_LINK
type is correct and the component receives all necessary props (data
,setData
, andpresets
) to function properly.components/links/link-sheet/index.tsx (1)
124-124
: LGTM: Type definition is properly added.The addition of
securedByPapermark: boolean
to the type definition is correct and maintains type safety.pages/api/links/[id]/index.ts (2)
49-49
: LGTM: Correctly added field to Prisma select queryThe addition of
securedByPapermark: true
to the select query ensures the new field is retrieved from the database.
334-334
: LGTM: Properly handled field update with default valueThe update operation correctly sets
securedByPapermark
with a fallback tofalse
if not provided, ensuring data consistency.components/view/viewer/dataroom-viewer.tsx (3)
101-101
: LGTM: Correctly added optional prop for secured by Papermark featureThe new
securedByPapermark
prop is properly typed as optional boolean, maintaining backward compatibility.Also applies to: 117-117
342-346
: LGTM: Smart height adjustment for footer accommodationThe dynamic height calculation properly accounts for the footer when
securedByPapermark
is enabled, preventing content from being hidden behind the fixed footer.
494-496
: LGTM: Proper conditional rendering of footer componentThe SecuredByPapermark component is correctly rendered only when the flag is enabled, with appropriate props passed.
components/view/secured-by-papermark.tsx (1)
24-24
: LGTM: Well-structured UTM tracking implementationThe UTM parameters are properly configured for campaign tracking, incorporating the linkId for analytics purposes.
components/links/link-sheet/secured-by-papermark-section.tsx (3)
20-32
: LGTM: Proper state synchronization with data and presetsThe useEffect hooks correctly handle:
- Synchronizing local state with the parent data
- Forcing the feature to be enabled when presets require it
The dependency arrays are properly configured to prevent unnecessary re-renders.
34-38
: LGTM: Well-implemented toggle handlerThe toggle handler properly updates both the local state and parent state, ensuring UI consistency.
40-50
: LGTM: Clean LinkItem integrationThe component properly integrates with the LinkItem component, providing appropriate tooltip content and ensuring the toggle is always allowed.
return createPortal( | ||
<div className="fixed bottom-0 left-0 right-0 z-[100] border-t border-gray-200 bg-white dark:border-gray-800 dark:bg-black"> | ||
<div className="flex items-center justify-between px-12 py-2 text-xs text-gray-500 dark:text-gray-400"> | ||
<div className="flex items-center space-x-2"> | ||
<span>© {new Date().getFullYear()} All rights reserved</span> | ||
</div> | ||
<div className="flex items-center space-x-1.5"> | ||
<span>Secured by</span> | ||
<a | ||
href={`https://www.papermark.com?utm_campaign=securedby&utm_medium=securedby&utm_source=papermark-${linkId}`} | ||
target="_blank" | ||
rel="noopener noreferrer" | ||
className="font-semibold text-gray-700 transition-colors hover:text-gray-900 dark:text-gray-300 dark:hover:text-gray-100" | ||
> | ||
Papermark | ||
</a> | ||
</div> | ||
{isClosable && ( | ||
<button | ||
onClick={() => setVisible(false)} | ||
className="absolute right-4 rounded text-lg text-gray-400 hover:text-gray-700 dark:hover:text-gray-200" | ||
aria-label="Close" | ||
> | ||
<CircleXIcon className="h-4 w-4" /> | ||
</button> | ||
)} | ||
</div> | ||
</div>, | ||
document.body, | ||
); |
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.
🛠️ Refactor suggestion
Add SSR safety check for document.body access
The component looks well-implemented with proper portal usage and styling. However, accessing document.body
directly can cause issues during server-side rendering.
Add a safety check to ensure the component only renders on the client side:
if (!visible) return null;
+ if (typeof window === 'undefined') return null;
return createPortal(
This prevents SSR errors when document.body
is not available.
📝 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.
return createPortal( | |
<div className="fixed bottom-0 left-0 right-0 z-[100] border-t border-gray-200 bg-white dark:border-gray-800 dark:bg-black"> | |
<div className="flex items-center justify-between px-12 py-2 text-xs text-gray-500 dark:text-gray-400"> | |
<div className="flex items-center space-x-2"> | |
<span>© {new Date().getFullYear()} All rights reserved</span> | |
</div> | |
<div className="flex items-center space-x-1.5"> | |
<span>Secured by</span> | |
<a | |
href={`https://www.papermark.com?utm_campaign=securedby&utm_medium=securedby&utm_source=papermark-${linkId}`} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="font-semibold text-gray-700 transition-colors hover:text-gray-900 dark:text-gray-300 dark:hover:text-gray-100" | |
> | |
Papermark | |
</a> | |
</div> | |
{isClosable && ( | |
<button | |
onClick={() => setVisible(false)} | |
className="absolute right-4 rounded text-lg text-gray-400 hover:text-gray-700 dark:hover:text-gray-200" | |
aria-label="Close" | |
> | |
<CircleXIcon className="h-4 w-4" /> | |
</button> | |
)} | |
</div> | |
</div>, | |
document.body, | |
); | |
if (!visible) return null; | |
if (typeof window === 'undefined') return null; | |
return createPortal( | |
<div className="fixed bottom-0 left-0 right-0 z-[100] border-t border-gray-200 bg-white dark:border-gray-800 dark:bg-black"> | |
<div className="flex items-center justify-between px-12 py-2 text-xs text-gray-500 dark:text-gray-400"> | |
<div className="flex items-center space-x-2"> | |
<span>© {new Date().getFullYear()} All rights reserved</span> | |
</div> | |
<div className="flex items-center space-x-1.5"> | |
<span>Secured by</span> | |
<a | |
href={`https://www.papermark.com?utm_campaign=securedby&utm_medium=securedby&utm_source=papermark-${linkId}`} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="font-semibold text-gray-700 transition-colors hover:text-gray-900 dark:text-gray-300 dark:hover:text-gray-100" | |
> | |
Papermark | |
</a> | |
</div> | |
{isClosable && ( | |
<button | |
onClick={() => setVisible(false)} | |
className="absolute right-4 rounded text-lg text-gray-400 hover:text-gray-700 dark:hover:text-gray-200" | |
aria-label="Close" | |
> | |
<CircleXIcon className="h-4 w-4" /> | |
</button> | |
)} | |
</div> | |
</div>, | |
document.body, | |
); |
🤖 Prompt for AI Agents
In components/view/secured-by-papermark.tsx around lines 15 to 44, the code
directly accesses document.body for createPortal, which causes errors during
server-side rendering because document is undefined on the server. To fix this,
add a client-side check to ensure the component only attempts to render the
portal when running in the browser, such as by using a state variable that is
set to true inside a useEffect hook or by checking if typeof window !==
'undefined' before accessing document.body. This will prevent SSR errors by
avoiding document access on the server.
Summary by CodeRabbit
New Features
Bug Fixes
Chores