-
Notifications
You must be signed in to change notification settings - Fork 0
[ADE-208] fixes for ADE-66 #138
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
…ry wrapper and adjust margin styles for better layout
…to enhance layout and readability
…ab to enhance layout and readability
…t and readability
… Notice component from AiAnalysisTab
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 PR addresses fixes for ADE-66 by adjusting the display of the AiAssistantNotice component, refining prop destructuring and formatting for consistency, and streamlining duplicate processing logic in the ProcessingPage.
- Removed the AiAssistantNotice import and instance from AiAnalysisTab while conditionally adding it to ReportDetailPage.
- Refactored component props syntax in ActionButtons and updated arrow function formatting in ReportsListPage.
- Simplified duplicate call prevention in ProcessingPage by removing the lastTriggeredTime variable.
Reviewed Changes
Copilot reviewed 25 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/pages/Reports/components/AiAnalysisTab.tsx | Removed unused AiAssistantNotice to clean up the UI. |
| frontend/src/pages/Reports/components/ActionButtons.tsx | Refactored prop destructuring for a cleaner and more compact syntax. |
| frontend/src/pages/Reports/ReportsListPage.tsx | Updated arrow function formatting to ensure consistency in filtering and mapping. |
| frontend/src/pages/Reports/ReportDetailPage.tsx | Conditionally renders AiAssistantNotice based on the activeTab state. |
| frontend/src/pages/Processing/ProcessingPage.tsx | Removed lastTriggeredTime variable; rely on clearStatusCheckInterval for status checks. |
| frontend/src/common/components/Upload/UploadModal.tsx | Adjusted text rendering in file details for better spacing. |
| frontend/src/common/components/Router/tests/TabNavigation.test.tsx | Added trailing commas in icon mocks for consistency. |
| frontend/src/common/components/Modal/ConfirmationModal.tsx | Simplified message formatting inline for consistency. |
| frontend/src/common/components/Icon/SvgIcon.tsx | Composed class names in a single line for greater readability. |
Files not reviewed (10)
- .husky/pre-commit: Language not supported
- backend/package.json: Language not supported
- frontend/package.json: Language not supported
- frontend/src/common/components/Icon/SvgIcon.scss: Language not supported
- frontend/src/common/components/Modal/ConfirmationModal.scss: Language not supported
- frontend/src/common/components/Router/TabNavigation.scss: Language not supported
- frontend/src/pages/Home/components/ReportItem/ReportItem.scss: Language not supported
- frontend/src/pages/Processing/ProcessingPage.scss: Language not supported
- frontend/src/pages/Reports/ReportsListPage.scss: Language not supported
- frontend/src/pages/Reports/components/AiAssistantNotice.scss: Language not supported
Comments suppressed due to low confidence (1)
frontend/src/pages/Processing/ProcessingPage.tsx:28
- The removal of the lastTriggeredTime variable simplifies the code; please ensure that the new approach adequately prevents duplicate API calls when processFile is triggered in rapid succession.
const lastTriggeredTime = useRef<number | null>(null);
Change
fixes for ADE-66
Does this PR introduce a breaking change?
{...}
What needs to be documented once your changes are merged?
{...}
Additional Comments
{...}