chat iframe revert #2737
chat iframe revert #2737
Conversation
|
WalkthroughThe pull request introduces several modifications across multiple components in the application, focusing on UI enhancements and logic adjustments. Key changes include updates to button styling in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 0
🧹 Outside diff range and nitpick comments (5)
apps/backoffice-v2/src/pages/Root/Root.page.tsx (1)
Line range hint
39-48: Consider cleaning up commented codeWhile not directly related to the main changes in this PR, I noticed that there's a commented-out
ReactQueryDevtoolscomponent in theRootcomponent. If this code is no longer needed, it would be good to remove it to keep the codebase clean.If the
ReactQueryDevtoolsis not going to be used, consider removing the commented code and the related import:- import React, { FunctionComponent, lazy, useState } from 'react'; + import React, { FunctionComponent, useState } from 'react'; - const ReactQueryDevtools = lazy(() => - process.env.NODE_ENV !== 'production' - ? import('@tanstack/react-query-devtools').then(module => ({ - default: module.ReactQueryDevtools, - })) - : Promise.resolve({ default: () => null }), - ); export const Root: FunctionComponent = () => { return ( <Providers> <ServerDownLayout> <Outlet /> </ServerDownLayout> <ChatbotLayout /> - {/*<Suspense>*/} - {/* <ReactQueryDevtools />*/} - {/*</Suspense>*/} </Providers> ); };If you plan to use
ReactQueryDevtoolsin the future, please add a TODO comment explaining why it's commented out and when it might be re-enabled.apps/backoffice-v2/src/lib/blocks/components/CaseCallToActionLegacy/CaseCallToActionLegacy.tsx (1)
135-135: Verify visual consistency and document style overrideThe addition of
!bg-foregroundto the button's className will change its background color and forcefully override any existing styles. While this change doesn't affect functionality, it may impact the visual consistency of the UI.
- Please verify that this style change aligns with the design system and doesn't create inconsistencies with similar buttons elsewhere in the application.
- Consider documenting the reason for this specific override, either in a comment or in the component's documentation, to provide context for future developers.
If this style change is intended to be applied more broadly, consider creating a new button variant or updating the existing styles in your CSS/Tailwind configuration instead of using an inline override.
apps/backoffice-v2/src/lib/blocks/components/KycBlock/hooks/useKycBlock/useKycBlock.tsx (3)
311-313: Improved button styling based on disabled stateThe conditional application of the
!bg-successclass when the button is not disabled is a good improvement for visual feedback. This change enhances the user experience by clearly indicating when the button is actionable.Consider extracting the className logic into a separate variable or utility function for better readability, especially if similar conditional styling is used elsewhere in the component.
const buttonClassName = ctw({ '!bg-success': !isDisabled, // Add any other conditional classes here });Then use it in the MotionButton:
className={buttonClassName}This approach would make the component more maintainable and easier to extend in the future.
Line range hint
193-270: Improved badge rendering logic for different statesThe refactoring of the
getDecisionStatusOrActionfunction significantly improves the clarity and maintainability of the code. Each state (REVISION, APPROVED, REJECTED, PENDING_PROCESS) is now explicitly handled, returning a badge with appropriate styling and text.To further improve the code, consider extracting the common badge creation logic into a separate function to reduce duplication. For example:
const createBadge = (value: string, variant: string, className: string) => createBlocksTyped() .addBlock() .addCell({ type: 'badge', value, props: { ...motionBadgeProps, variant, className: `${badgeClassNames} ${className}`, }, }) .build() .flat(1); // Usage if (tags?.includes(StateTag.APPROVED)) { return createBadge('Approved', 'success', 'bg-success/20'); }This refactoring would make the function more concise and easier to maintain, especially if you need to add more states in the future.
Line range hint
1-576: Overall improvements to useKycBlock hookThe changes to the
useKycBlockhook have enhanced its functionality without altering its interface. The improvements in data processing and rendering, particularly in handling different KYC verification states, contribute to a more robust and flexible component.For future iterations, consider breaking down this large hook into smaller, more focused custom hooks. This could improve maintainability and testability. For example:
useKycDocumentsfor handling document-related logicuseKycDecisionfor decision-making logicuseKycBadgesfor badge rendering logicThis separation of concerns would make the code easier to understand and maintain as the component grows in complexity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- apps/backoffice-v2/src/lib/blocks/components/CaseCallToActionLegacy/CaseCallToActionLegacy.tsx (1 hunks)
- apps/backoffice-v2/src/lib/blocks/components/KycBlock/hooks/useKycBlock/useKycBlock.tsx (1 hunks)
- apps/backoffice-v2/src/pages/Entities/components/CaseCreation/components/CaseCreationForm/components/SubmitSection/SubmitSection.tsx (1 hunks)
- apps/backoffice-v2/src/pages/Root/Root.page.tsx (1 hunks)
- services/workflows-service/prisma/data-migrations (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- services/workflows-service/prisma/data-migrations
🔇 Additional comments (2)
apps/backoffice-v2/src/pages/Entities/components/CaseCreation/components/CaseCreationForm/components/SubmitSection/SubmitSection.tsx (1)
23-23: Approve with suggestions: Document the style override and verify accessibilityThe addition of the
!bg-foregroundclass to the Button component is approved as it doesn't introduce any functional issues. However, there are a few points to consider:
The use of the
!important override might lead to maintenance challenges in the future. Consider documenting the reason for this override in a comment.Changing the button's background color could impact its visibility and contrast ratio. Please verify that the button still meets accessibility standards (WCAG 2.1 AA) with this new style.
It's recommended to test the button's appearance across different themes or color schemes to ensure it remains visible and functional in all contexts.
To verify the impact of this change, please run the following accessibility check:
apps/backoffice-v2/src/pages/Root/Root.page.tsx (1)
32-32: Chatbot rendering simplified, potential impacts to considerThe change simplifies the
ChatbotLayoutcomponent by directly rendering theChatbotcomponent instead of wrapping it in aRenderChildrenInIFrame. This aligns with the PR title "chat iframe revert".While this simplification can improve performance and reduce complexity, please consider the following:
- If the iframe was used for security isolation, ensure that removing it doesn't introduce any security vulnerabilities.
- Verify that the
Chatbotcomponent still functions correctly without the iframe wrapper, especially regarding its positioning and interaction with other elements on the page.- Confirm that this change doesn't affect any CSS styling that might have been relying on the iframe structure.
To ensure no unintended side-effects, please run the following script to check for any remaining references to the removed iframe or related styling:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style