Add iframe fixes #504
Add iframe fixes #504
Conversation
[TASK-6625 & 6622] fix kyc iframe dimensions & cose mechanism
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
src/components/Global/KYCComponent/index.tsx (2)
194-194: Consider improving the confirmation message format.The confirmation message could be more professional and properly formatted.
Consider this improvement:
- setIframeOptions({ ...iframeOptions, src: kyclink, visible: true, closeConfirmMessage: 'Are you sure ? your KYC progress will be lost.' }) + setIframeOptions({ ...iframeOptions, src: kyclink, visible: true, closeConfirmMessage: 'Are you sure you want to exit? Your KYC verification progress will be lost.' })
401-401: Consider documenting expected IframeWrapper props.While spreading props is clean, consider adding a comment documenting the expected props for better maintainability.
Add a comment above the IframeWrapper:
// Expected props: src, visible, onClose, closeConfirmMessage <IframeWrapper {...iframeOptions} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Global/IframeWrapper/index.tsx(3 hunks)src/components/Global/KYCComponent/index.tsx(4 hunks)
🔇 Additional comments (7)
src/components/Global/IframeWrapper/index.tsx (4)
4-9: LGTM! Clean type definition.
The IFrameWrapperProps type is well-structured and properly exported for reuse.
57-66: LGTM! Well-documented modal close handling.
The confirmation logic is well-implemented with clear documentation.
89-93: LGTM! Secure iframe configuration.
The iframe configuration includes appropriate sandbox attributes and security measures.
Line range hint 17-34: Verify origin check implementation.
The message event handler correctly uses window.location.origin for security. However, let's verify there are no other message handlers that might conflict with this implementation.
✅ Verification successful
Message event handler implementation is secure and isolated
The verification confirms this is the only message event handler in the codebase, and it correctly implements origin validation using window.location.origin. The implementation is safe and doesn't have any conflicting handlers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other message event handlers in the codebase
rg -g '*.{ts,tsx,js,jsx}' -B 2 -A 2 'addEventListener\(['"'"']message['"'"']'
Length of output: 400
src/components/Global/KYCComponent/index.tsx (3)
9-9: LGTM! Good type safety improvement.
The addition of the IFrameWrapperProps type import enhances type safety and provides better IDE support.
Line range hint 36-42: LGTM! Proper type usage for state management.
The state initialization with IFrameWrapperProps type ensures type safety and the initial values are properly set with a working close handler.
194-194: Verify closeConfirmMessage behavior in all KYC scenarios.
Let's ensure the confirmation message is consistently applied in all KYC-related scenarios.
✅ Verification successful
Let me gather more information about the iframe implementation to ensure comprehensive verification.
Let me check the IFrameWrapper component implementation to verify how the closeConfirmMessage is handled.
Let me try a different approach to find the IFrameWrapper implementation.
Let me check the complete implementation of IframeWrapper to verify how the confirmation message is handled.
Close confirmation dialog is properly implemented for KYC iframe
The closeConfirmMessage implementation in the IFrameWrapper component is robust and properly handles the KYC scenario:
- Confirmation dialog is shown when closeConfirmMessage is provided
- Both mobile and desktop layouts are supported
- Dialog prevents accidental closure through overlay clicks
- Implementation maintains proper state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other KYC-related iframe usage that might need the confirmation message
rg -l "setIframeOptions.*visible:\s*true" | xargs rg "kyc"
Length of output: 856
Script:
#!/bin/bash
# Search for IframeOptions type definition and usage
ast-grep --pattern 'interface IframeOptions {
$$$
}'
# Also check for other setIframeOptions usage
rg "setIframeOptions" -A 2 -B 2
Length of output: 1830
Script:
#!/bin/bash
# Search for IFrameWrapper component implementation
ast-grep --pattern 'const IFrameWrapper = ({ $$ }: IFrameWrapperProps) => {
$$$
}'
# Also search for IFrameWrapperProps type definition
ast-grep --pattern 'interface IFrameWrapperProps {
$$$
}'
Length of output: 148
Script:
#!/bin/bash
# Search for files containing IFrameWrapper
fd IFrameWrapper
# Search for files containing IFrameWrapperProps
rg "IFrameWrapperProps" -A 5 -B 5
Length of output: 3554
Script:
#!/bin/bash
# Get the complete implementation of IframeWrapper component
cat src/components/Global/IframeWrapper/index.tsx
Length of output: 3934
| <div className="flex flex-col h-full gap-2 p-0 sm:p-2"> | ||
| <div className="w-full sm:hidden"> | ||
| <button className="btn-purple w-full rounded-none" onClick={onClose}> | ||
| {enableConfirmationPrompt ? areYouSurePromptRow : <button className="btn-purple w-full rounded-none" onClick={() => { | ||
| onClose() | ||
| }}> | ||
| CLOSE | ||
| </button> | ||
| </button>} | ||
| </div> | ||
| <div className="h-[550px] overflow-hidden rounded-sm sm:h-[500px] sm:border sm:border-black"> | ||
| <div className="overflow-hidden h-full"> | ||
| <iframe | ||
| src={src} | ||
| allow="camera;" | ||
| style={{ ...style, width: '100%', height: '100%', border: 'none' }} | ||
| style={{ width: '100%', height: '100%', border: 'none' }} | ||
| className="rounded-md" | ||
| sandbox="allow-same-origin allow-scripts allow-forms allow-popups allow-modals allow-top-navigation-by-user-activation" | ||
| /> | ||
| </div> | ||
| <div className="hidden w-full sm:flex"> | ||
| <button className="btn-purple w-full" onClick={onClose}> | ||
| Close | ||
| </button> | ||
| <div className="hidden sm:flex flex-row w-full items-center"> | ||
| {enableConfirmationPrompt ? areYouSurePromptRow : <button className="btn-purple w-full rounded-none" onClick={() => { | ||
| onClose() | ||
| }}> | ||
| CLOSE | ||
| </button>} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating mobile and desktop view logic.
The close button implementation is duplicated between mobile and desktop views. Consider using CSS to handle the visibility instead of duplicating the JSX.
-<div className="w-full sm:hidden">
- {enableConfirmationPrompt ? areYouSurePromptRow : <button className="btn-purple w-full rounded-none" onClick={() => {
- onClose()
- }}>
- CLOSE
- </button>}
-</div>
-// ... iframe ...
-<div className="hidden sm:flex flex-row w-full items-center">
- {enableConfirmationPrompt ? areYouSurePromptRow : <button className="btn-purple w-full rounded-none" onClick={() => {
- onClose()
- }}>
- CLOSE
- </button>}
-</div>
+<div className="w-full sm:hidden">
+ <CloseButtonSection
+ enableConfirmationPrompt={enableConfirmationPrompt}
+ areYouSurePromptRow={areYouSurePromptRow}
+ onClose={onClose}
+ />
+</div>
+// ... iframe ...
+<div className="hidden sm:flex flex-row w-full items-center">
+ <CloseButtonSection
+ enableConfirmationPrompt={enableConfirmationPrompt}
+ areYouSurePromptRow={areYouSurePromptRow}
+ onClose={onClose}
+ />
+</div>Committable suggestion was skipped due to low confidence.
| const areYouSurePromptRow = showCloseConfirmMessage ? ( | ||
| <div className='flex flex-col text-center sm:text-left sm:flex-row justify-between sm:items-center gap-4 p-2 sm:p-0 w-full'> | ||
| <p className="text-sm ml-1">{closeConfirmMessage}</p> | ||
| <div className="flex flex-row items-center gap-2"> | ||
| <button className='btn-stroke w-full' onClick={() => { | ||
| setShowCloseConfirmMessage(false) | ||
| }}>Cancel</button> | ||
| <button className='btn-purple w-full' onClick={() => { | ||
| onClose() | ||
| setShowCloseConfirmMessage(false) | ||
| }}>Close</button> | ||
| </div> | ||
| </div> | ||
| ) : <button className="btn-purple w-full rounded-none" onClick={() => { | ||
| setShowCloseConfirmMessage(true) | ||
| }}> | ||
| CLOSE | ||
| </button> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting close button to reduce duplication.
The close button implementation is repeated multiple times with similar logic. Consider extracting it into a reusable component.
+const CloseButton = ({ onClose }: { onClose: () => void }) => (
+ <button className="btn-purple w-full rounded-none" onClick={onClose}>
+ CLOSE
+ </button>
+);
const IframeWrapper = ({
// ... props
}) => {
// ... other code
const areYouSurePromptRow = showCloseConfirmMessage ? (
// ... confirmation prompt JSX
- ) : <button className="btn-purple w-full rounded-none" onClick={() => {
- setShowCloseConfirmMessage(true)
- }}>
- CLOSE
- </button>
+ ) : <CloseButton onClose={() => setShowCloseConfirmMessage(true)} />Committable suggestion was skipped due to low confidence.
No description provided.