-
Notifications
You must be signed in to change notification settings - Fork 8
[CMG-726] - The “Overlapping” and “Advanced Settings” buttons are not responding on multiple clicks. #810
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
Conversation
…tack/migration-v2 into bugfix/sitecore-zip
…de-server into feature/aem-final
…de-server into feature/aem-final
…de-server into feature/aem-final
…nd error handling. Standardize string quotes, enhance file format validation, and add console warnings for incomplete steps in Legacy CMS. Update configuration for local path.
…to bugfix/sitecore-zip-issue
…ependencies for better clarity in development environment
…thods; streamline ESLint configuration
…son and package-lock.json to latest versions for improved functionality and security
Bugfix/sitecore zip issue
CMG-752: Fix advanced settings not resetting on mapping reset
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.
Pull Request Overview
This pull request makes UI improvements and code cleanup across multiple components. The main focus is on standardizing notification behavior and fixing UI event handlers.
- Standardizes notification behavior by hiding progress bars across the application
- Fixes event handler placement by moving
onClickfrom Icon to Button components - Removes commented-out code and unused imports
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/pages/Migration/index.tsx | Sets hideProgressBar: true for migration execution notification |
| ui/src/pages/Login/index.tsx | Adds hideProgressBar: true to TFA success notification and removes commented-out useEffect code |
| ui/src/components/Modal/index.tsx | Adds hideProgressBar: true to both success and error notifications |
| ui/src/components/LogScreen/MigrationLogViewer.tsx | Removes unused imports for updateCurrentStepData, ModalObj, and MigrationCompletionModal |
| ui/src/components/ContentMapper/index.tsx | Moves onClick handlers from Icon to parent Button components (2 instances) |
| ui/src/components/ContentMapper/index.scss | Adjusts padding for advanced-setting-button from 5px to 5px 10px |
| ui/src/components/AdvancePropertise/index.tsx | Adds conditional logic for Reference field options and disables checkbox for Modular Blocks/Block field types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (Array.isArray(props?.data?.referenceTo) && props?.data?.referenceTo?.length) ? [props.data.referenceTo.map((item: any) => ({ | ||
| label: item, | ||
| value: item | ||
| })), ...option] | ||
| : option ?? []} |
Copilot
AI
Nov 4, 2025
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.
The mapped referenceTo array is wrapped in an extra array bracket, creating a nested array structure [[{label, value}, ...], ...option] instead of the intended flat array [{label, value}, ..., ...option]. Remove the outer brackets around props.data.referenceTo.map(...) to properly spread the mapped items.
| }} | ||
| options={ | ||
| option} | ||
| (Array.isArray(props?.data?.referenceTo) && props?.data?.referenceTo?.length) ? [props.data.referenceTo.map((item: any) => ({ |
Copilot
AI
Nov 4, 2025
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.
Replace any type with a more specific type definition. The item parameter should have an explicit type for better type safety.
| (Array.isArray(props?.data?.referenceTo) && props?.data?.referenceTo?.length) ? [props.data.referenceTo.map((item: any) => ({ | |
| (Array.isArray(props?.data?.referenceTo) && props?.data?.referenceTo?.length) ? [props.data.referenceTo.map((item: string) => ({ |
…or improved data handling
…hould be prevented.
…hould be prevented.
…e component; add effect for updating referenced items
refactor: add isNumber field to SearchComponent and TeaserComponent f…
No description provided.