[Tech] Migre monitor-ui vers la v13 #2979
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 27, 2024
Merged
Merged
ivangabriele
force-pushed
the
ivan/upgrade-monitor-ui-to-v12
branch
7 times, most recently
from
11f7562
to
0a4e552
Compare
This was referenced Feb 28, 2024
Merged
Merged
ivangabriele
force-pushed
the
ivan/upgrade-monitor-ui-to-v12
branch
from
b9537ee
to
aafdb6f
Compare
louptheron
reviewed
Feb 29, 2024
There was a problem hiding this comment.
Bien joué ça rend bien globalement !
Deux retours :
- Ce n'est plus possible de filtrer par zone dans la liste des navires, la sélection d'une zone ne l'enregistre pas dans le Cascader :
- Le formulaire des signalements est cassé, mais pas besoin de le fixer car je travaille actuellement dessus :
louptheron
reviewed
Feb 29, 2024
| @@ -6,56 +6,56 @@ import type { TableOptions } from '../../../../hooks/useTable/types' | |||
| export const REPORTING_LIST_TABLE_OPTIONS: TableOptions<InfractionSuspicionReporting | PendingAlertReporting> = { | |||
| columns: [ | |||
| { | |||
| fixedWidth: 112, | |||
| fixedWidth: 130, | |||
There was a problem hiding this comment.
Pourquoi augmenter ces valeurs ? Il faut faire attention à ce que ça convienne sur les postes des utilisateurs
There was a problem hiding this comment.
Ça correspond, le tableau est même un poil moins large qu'avant, on a gagné des paddings qui devaient provenir des styles globaux je pense.
louptheron
reviewed
Feb 29, 2024
| @@ -296,11 +297,11 @@ const columnStyles: CSSProperties[] = [ | |||
| }, | |||
| { | |||
| ...styleCenter, | |||
| width: 112 | |||
| width: 130 | |||
There was a problem hiding this comment.
Pareil pour le width et la résolution des écrans utilisateurs
ivangabriele
changed the title
WalkthroughThe changes encompass a wide array of updates across the frontend, focusing on enhancing testing capabilities, improving user interface interactions, and refining styles for consistency. Key updates include adjustments to Cypress e2e tests, modifications in component styling, and better alignment with updated selectors and components for a smoother user experience. These adjustments aim to streamline development workflows and ensure a more robust and user-friendly application. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 13
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by:!**/*.jsonfrontend/package.jsonis excluded by:!**/*.json
Files selected for processing (77)
- frontend/config/jest.config.js (1 hunks)
- frontend/cypress/e2e/backoffice/fleet_segments.spec.ts (8 hunks)
- frontend/cypress/e2e/backoffice/new_regulation.spec.ts (2 hunks)
- frontend/cypress/e2e/backoffice/update_regulation.spec.ts (4 hunks)
- frontend/cypress/e2e/side_window/alert_list/alerts_list.spec.ts (1 hunks)
- frontend/cypress/e2e/side_window/alert_list/utils.ts (1 hunks)
- frontend/cypress/e2e/side_window/beacon_malfunction/board.spec.ts (6 hunks)
- frontend/cypress/e2e/side_window/beacon_malfunction/utils.ts (1 hunks)
- frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts (2 hunks)
- frontend/cypress/e2e/side_window/mission_form/land_control.spec.ts (1 hunks)
- frontend/cypress/e2e/side_window/mission_form/main_form.spec.ts (6 hunks)
- frontend/cypress/e2e/side_window/mission_form/sea_control.spec.ts (5 hunks)
- frontend/cypress/e2e/side_window/mission_form/utils.ts (2 hunks)
- frontend/cypress/e2e/side_window/mission_list/export_activity_reports.spec.ts (1 hunks)
- frontend/cypress/e2e/side_window/mission_list/filter_bar.spec.ts (1 hunks)
- frontend/cypress/e2e/side_window/mission_list/utils.ts (1 hunks)
- frontend/cypress/e2e/side_window/reporting_list.spec.ts (1 hunks)
- frontend/cypress/e2e/vessel_sidebar/logbook.spec.ts (2 hunks)
- frontend/cypress/e2e/vessels/vessel_filters.spec.ts (1 hunks)
- frontend/cypress/e2e/vessels/vessel_track.spec.ts (1 hunks)
- frontend/src/App.tsx (2 hunks)
- frontend/src/components/CustomGlobalStyle.ts (1 hunks)
- frontend/src/features/ActivityReport/components/ExportActivityReportsDialog/index.tsx (3 hunks)
- frontend/src/features/Backoffice/edit_regulation/gear_regulation/RegulatedGears.jsx (1 hunks)
- frontend/src/features/Backoffice/fleet_segments/NewFleetSegmentModal.tsx (4 hunks)
- frontend/src/features/ControlUnit/components/ControlUnitListDialog/FilterBar.tsx (4 hunks)
- frontend/src/features/Logbook/components/VesselLogbook/LogbookSummary/index.tsx (1 hunks)
- frontend/src/features/MainWindow.tsx (2 hunks)
- frontend/src/features/MapButtons/index.tsx (2 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/AirSurveillanceForm.tsx (3 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FleetSegmentsField.tsx (3 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FormikMultiInfractionPicker/InfractionForm.tsx (4 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FormikPortSelect.tsx (3 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/GearsField.tsx (3 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/SpeciesField.tsx (4 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionForm/shared/VesselField.tsx (1 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionList/Item.tsx (1 hunks)
- frontend/src/features/Mission/components/MissionForm/ActionList/utils.tsx (1 hunks)
- frontend/src/features/Mission/components/MissionForm/MainForm/FormikDoubleDatePicker.tsx (2 hunks)
- frontend/src/features/Mission/components/MissionForm/MainForm/FormikMultiControlUnitPicker/ControlUnitSelect.tsx (5 hunks)
- frontend/src/features/Mission/components/MissionForm/constants.tsx (2 hunks)
- frontend/src/features/Reporting/components/ReportingList/constants.ts (1 hunks)
- frontend/src/features/Reporting/components/ReportingList/index.tsx (4 hunks)
- frontend/src/features/SideWindow/Alert/AlertListAndReportingList/PendingAlertRow.tsx (1 hunks)
- frontend/src/features/SideWindow/Alert/AlertListAndReportingList/PendingAlertsList.tsx (1 hunks)
- frontend/src/features/SideWindow/Alert/SilencedAlerts/fields/AlertTypeField.tsx (3 hunks)
- frontend/src/features/SideWindow/Alert/SilencedAlerts/index.tsx (1 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionCard.tsx (4 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionDetails.tsx (2 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionDetailsFollowUp.tsx (5 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/SendNotification.tsx (3 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/VesselStatusSelectValue.tsx (1 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/index.tsx (5 hunks)
- frontend/src/features/SideWindow/MissionList/FilterBar.tsx (6 hunks)
- frontend/src/features/SideWindow/SubMenu/Item.tsx (1 hunks)
- frontend/src/features/SideWindow/SubMenu/index.tsx (2 hunks)
- frontend/src/features/VesselList/VesselListFilters.tsx (4 hunks)
- frontend/src/features/VesselList/index.tsx (3 hunks)
- frontend/src/features/VesselList/tableCells.tsx (2 hunks)
- frontend/src/features/VesselSidebar/actions/TrackRequest/DateRangeRadio.tsx (1 hunks)
- frontend/src/features/VesselSidebar/actions/TrackRequest/ExportTrack.tsx (2 hunks)
- frontend/src/features/VesselSidebar/actions/TrackRequest/PositionsTable.tsx (6 hunks)
- frontend/src/features/VesselSidebar/actions/TrackRequest/index.tsx (4 hunks)
- frontend/src/features/VesselSidebar/beacon_malfunctions/details/BeaconMalfunctionDetails.tsx (1 hunks)
- frontend/src/features/VesselSidebar/beacon_malfunctions/resume/CurrentBeaconMalfunctionBody.tsx (2 hunks)
- frontend/src/features/VesselSidebar/beacon_malfunctions/resume/YearsToBeaconMalfunctionList.tsx (1 hunks)
- frontend/src/features/commonComponents/StyledModalHeader.jsx (1 hunks)
- frontend/src/features/map/draw/DrawModal.tsx (2 hunks)
- frontend/src/features/map/layers/Mission/MissionsLabelsLayer/MissionsLabelsLayer.tsx (2 hunks)
- frontend/src/features/map/layers/Mission/MissionsLabelsLayer/types.ts (1 hunks)
- frontend/src/features/map/layers/Mission/MissionsLabelsLayer/utils.ts (2 hunks)
- frontend/src/hooks/useTable/TableHeadNext.tsx (1 hunks)
- frontend/src/index.tsx (1 hunks)
- frontend/src/pages/BackofficePage.tsx (2 hunks)
- frontend/src/ui/LegacyRsuiteComponentsWrapper.tsx (1 hunks)
- frontend/src/ui/NoRsuiteOverrideWrapper.tsx (1 hunks)
- frontend/src/ui/card-table/CardTableRow.tsx (1 hunks)
Files skipped from review due to trivial changes (6)
- frontend/src/features/Backoffice/edit_regulation/gear_regulation/RegulatedGears.jsx
- frontend/src/features/SideWindow/Alert/SilencedAlerts/index.tsx
- frontend/src/features/SideWindow/SubMenu/Item.tsx
- frontend/src/features/VesselSidebar/actions/TrackRequest/ExportTrack.tsx
- frontend/src/features/VesselSidebar/beacon_malfunctions/details/BeaconMalfunctionDetails.tsx
- frontend/src/features/VesselSidebar/beacon_malfunctions/resume/YearsToBeaconMalfunctionList.tsx
Additional comments: 121
frontend/src/features/map/layers/Mission/MissionsLabelsLayer/types.ts (1)
- 1-7: The
FeatureAndLabeltype definition looks good and adheres to TypeScript best practices.frontend/src/ui/card-table/CardTableRow.tsx (1)
- 16-16: The change in the
heightproperty ofCardTableRowto42pxcan improve readability. Please ensure this change aligns with the design specifications for consistency across the UI.frontend/src/features/commonComponents/StyledModalHeader.jsx (1)
- 7-12: The updates to
StyledModalHeaderusing$isFullfor conditional styling adhere to styled-components conventions and improve the component's flexibility.frontend/src/ui/NoRsuiteOverrideWrapper.tsx (1)
- 46-49: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
frontend/src/features/SideWindow/SubMenu/index.tsx (1)
- 30-35: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
frontend/src/features/Mission/components/MissionForm/MainForm/FormikDoubleDatePicker.tsx (1)
- 47-48: The adjustments to the styling of date pickers in
FormikDoubleDatePickerare well-considered and improve the component's layout.frontend/cypress/e2e/side_window/mission_list/utils.ts (1)
- 7-7: The addition of the viewport setting before visiting the URL in
openSideWindowMissionListis a good practice for ensuring consistent test environments.frontend/config/jest.config.js (1)
- 24-24: The addition of the
@storemapping in the Jest configuration is correctly implemented and will simplify import statements in tests.frontend/src/index.tsx (1)
- 7-26: The updates related to importing
getOIDCConfigand using the nullish coalescing operator for environment variables are correctly implemented and improve the application's initialization process.frontend/src/pages/BackofficePage.tsx (1)
- 34-40: The use of
LegacyRsuiteComponentsWrapperto wrapBackOfficeMenuandOutletcomponents is a good strategy for handling legacy support and ensuring compatibility with the updated version ofmonitor-ui.frontend/src/features/VesselSidebar/actions/TrackRequest/DateRangeRadio.tsx (1)
- 57-61: The styling adjustments to
StyledRadiocomponents withinDateRangeRadioimprove visual spacing and alignment, enhancing the component's usability and appearance.frontend/cypress/e2e/vessels/vessel_filters.spec.ts (1)
- 15-15: Updating the selector to be more specific is a good practice that likely improves the reliability of the E2E test by ensuring the correct element is targeted.
frontend/src/features/map/layers/Mission/MissionsLabelsLayer/utils.ts (2)
- 4-4: Adding type annotations improves code readability and type safety. Good job on this enhancement.
- 50-57: The added check for
previousFeaturesAndLabelsbefore proceeding with the function's logic is a good practice to avoid potential runtime errors. This is a positive change.frontend/src/features/Mission/components/MissionForm/ActionList/utils.tsx (1)
- 15-34: The addition of the
subjectparameter to thegetActionTitlefunction and its conditional rendering improves the display of action titles, making them more informative. This is a positive change that enhances the component's functionality and user experience.frontend/cypress/e2e/side_window/mission_form/utils.ts (2)
- 6-7: Setting the viewport size before navigating to the side window is a good practice for ensuring consistency in the test environment. This enhances the reliability and consistency of the tests.
- 25-26: The inclusion of the
cy.viewportcall in theeditSideWindowMissionfunction, similar to theopenSideWindowNewMissionfunction, standardizes the viewport size for the test, contributing to more reliable and consistent test execution.frontend/src/hooks/useTable/TableHeadNext.tsx (1)
- 93-93: Changing the padding from
0.25remto4pxis a minor styling adjustment that likely aims to standardize the padding across different screen sizes and resolutions. This change could make the padding more predictable in environments where the root font size may vary.frontend/src/features/Mission/components/MissionForm/constants.tsx (2)
- 3-7: The updates to imports, including the corrected path for
LegacyControlUnitand the addition ofOptionfrom@mtes-mct/monitor-ui, improve the code's clarity and maintainability.- 61-65: The introduction of the
CONTROL_CHECKS_AS_OPTIONSconstant enhances code readability and reusability by providing a centralized definition for control check options. This is a positive change.frontend/src/App.tsx (1)
- 1-3: Replacing
OnlyFontGlobalStylewithGlobalStyleand addingCustomGlobalStyleenhances the application's overall styling by providing a more comprehensive set of global styles and allowing for custom styling adjustments. This is a positive change.frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FormikPortSelect.tsx (1)
- 3-3: Removing
useNewWindowand related references simplifies the component's logic and reduces dependencies on specific window management hooks, which could improve maintainability and performance.frontend/src/features/MapButtons/index.tsx (1)
- 43-53: Wrapping the content with
LegacyRsuiteComponentsWrapperensures compatibility with legacy components and styling, enhancing the consistency of styling and functionality across different versions of the UI library. This is a positive change.frontend/src/features/Mission/components/MissionForm/ActionForm/AirSurveillanceForm.tsx (1)
- 55-55: The refactor of
FormikMultiSelectto remove thebaseContainerprop and inline its props forisLight,label,name, andoptionsis a positive change. It simplifies the component's usage and makes the props more explicit, enhancing readability and maintainability.frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FormikMultiInfractionPicker/InfractionForm.tsx (1)
- 76-76: The removal of the
baseContainerprop from theHackedFormikSelectcomponent aligns with the removal ofuseNewWindowhook. This change likely reflects a shift in how new window containers are managed or a simplification of the component's usage. Ensure that this change does not affect the functionality or layout of theHackedFormikSelectcomponent in different contexts.frontend/cypress/e2e/side_window/mission_list/filter_bar.spec.ts (1)
- 66-66: The modification to directly fill the 'Unité' field with 'BSL Lorient' instead of interacting with checkboxes simplifies the test and makes it more straightforward. This change should make the test more reliable by reducing the steps involved and potentially avoiding issues with checkbox interactions. Ensure that this change accurately reflects the intended user interaction and that the test still covers the necessary functionality.
frontend/src/features/MainWindow.tsx (1)
- 64-71: The introduction of the
LegacyRsuiteComponentsWrapperto encapsulate components likeLayersSidebar,VesselSidebarHeader,MapButtons,RightMenuOnHoverArea,VesselList, andVesselSidebaris a significant structural change. This wrapper likely serves to manage legacy components more effectively or to apply consistent styling or behavior. Ensure that this change does not introduce any layout or functionality issues and that all encapsulated components function as expected within the new wrapper.frontend/src/features/Mission/components/MissionForm/ActionForm/shared/FleetSegmentsField.tsx (1)
- 3-3: The removal of
useNewWindowfromFleetSegmentsField.tsxsuggests a change in how new window containers are managed or possibly a simplification of the component's functionality. This change should not affect the core functionality of theFleetSegmentsFieldcomponent, but it's important to ensure that any related functionality that might have depended onuseNewWindowis still correctly implemented.frontend/src/features/VesselSidebar/beacon_malfunctions/resume/CurrentBeaconMalfunctionBody.tsx (1)
- 36-36: The adjustment of the margin property for a selector from '0 45px 0 0' to '0 16px 0 0' is a styling change that likely aims to improve the layout or visual appearance of the component. Ensure that this change positively affects the component's layout and does not introduce any alignment or spacing issues with adjacent elements.
frontend/src/features/ActivityReport/components/ExportActivityReportsDialog/index.tsx (3)
- 75-75: The addition of the
name="afterDateTimeUtc"property to the start date picker is a good practice, as it explicitly associates the input field with its corresponding value in the form data. This change enhances the form's accessibility and maintainability.- 86-86: Similarly, adding the
name="beforeDateTimeUtc"property to the end date picker improves form data association and accessibility. It's important to maintain consistency in naming conventions and ensure that these names accurately reflect their purpose and usage within the form.- 107-107: Modifying the
valueprop for the JDP select component to remove the|| undefinedpart simplifies the code and ensures that thevalueis always explicitly set to the current state or undefined. This change likely improves the component's predictability and state management.frontend/src/features/ControlUnit/components/ControlUnitListDialog/FilterBar.tsx (4)
- 93-93: The prop
isLighthas been changed toisTransparentin theTextInputcomponent. Ensure that this change aligns with the intended visual design and that theTextInputcomponent supports theisTransparentprop.- 103-103: The prop
isLighthas been changed toisTransparentin the firstSelectcomponent. Verify that this change is consistent with the component's design expectations and that theSelectcomponent properly supports theisTransparentprop.- 114-114: The prop
isLighthas been changed toisTransparentin the secondSelectcomponent. Confirm that this modification matches the desired visual styling and that theSelectcomponent adequately supports theisTransparentprop.- 125-125: The prop
isLighthas been changed toisTransparentin the thirdSelectcomponent. Ensure this alteration is in line with the visual design goals and that theSelectcomponent correctly supports theisTransparentprop.frontend/src/features/Mission/components/MissionForm/ActionForm/shared/VesselField.tsx (1)
- 83-83: The
handleIsVesselUnknownChangefunction now acceptsundefinedas a parameter, enhancing flexibility in handling the unknown vessel state. Ensure this change does not introduce unintended behavior in the component's functionality.frontend/src/features/SideWindow/BeaconMalfunctionBoard/SendNotification.tsx (1)
- 1-1: The removal of
useNewWindowand related references inSendNotificationsimplifies the component. Verify that this change does not negatively impact any features or user experience that relied onuseNewWindow.frontend/src/features/map/layers/Mission/MissionsLabelsLayer/MissionsLabelsLayer.tsx (2)
- 17-17: The import of
FeatureAndLabelsuggests new functionality or types are being utilized inMissionsLabelsLayer. Ensure this import is used effectively within the component.- 91-98: The modification to include
_previousFeaturesAndLabelsin theaddLabelsToAllFeaturesInExtentfunction's arguments allows for enhanced label management. Verify that this change is effectively utilized to improve the component's functionality.frontend/src/features/VesselSidebar/actions/TrackRequest/index.tsx (3)
- 101-101: The use of the nullish coalescing operator for
defaultValueinDateRangeRadioensures a more robust handling of default values. Confirm that this change aligns with the intended behavior for default value assignment.- 113-113: The addition of the
name="dateRange"attribute to theDateRangePickercomponent enhances form handling by providing a clear identifier for the date range input. Ensure this attribute is correctly utilized in form submissions and validations.- 128-128: Adjustments to padding values in styled components aim to improve the visual design and spacing within the component. Verify that these changes result in the desired visual appearance and do not introduce any layout issues.
frontend/src/features/Mission/components/MissionForm/ActionForm/shared/GearsField.tsx (1)
- 10-10: The removal of
useNewWindowand thebaseContainerprop in theSelectcomponent simplifies theGearsFieldcomponent. Verify that this change does not negatively impact any features or user experience that relied onuseNewWindowor thebaseContainerprop.frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts (2)
- 23-23: The assertion checks for the presence of 'Note libre à renseigner' within the first action list item, which aligns with the intended positive check for text content. This change enhances test clarity by explicitly verifying expected content.
- 34-34: The assertion verifies the presence of 'Une observation.' within the first action list item. This change from a negative to a positive check ensures the test accurately reflects the UI behavior and expected outcomes.
frontend/src/features/Mission/components/MissionForm/ActionForm/shared/SpeciesField.tsx (4)
- 11-11: The removal of an unused import (
useNewWindow) is a good practice for maintaining clean and efficient code. It helps reduce the bundle size and improves readability.- 18-18: Replacing hardcoded options with a constant (
CONTROL_CHECKS_AS_OPTIONS) improves maintainability and readability. It centralizes the options, making future changes easier and reducing the risk of inconsistencies.- 148-148: The use of
CONTROL_CHECKS_AS_OPTIONSfor options in theFormikMultiRadiocomponent is a good practice. It ensures consistency and maintainability by using a centralized source for options.- 183-183: Removing the
baseContainerprop from theSelectcomponent, assuming it was unused or unnecessary, is a positive change for code cleanliness. It's important to ensure that the removal does not affect the component's functionality or styling.frontend/cypress/e2e/side_window/alert_list/alerts_list.spec.ts (2)
- 1-1: Adding an import statement for
openSideWindowAlertListfrom './utils' is a good practice. It promotes code reuse and maintainability by utilizing a utility function for common setup steps in tests.- 6-6: Replacing
cy.visit('/side_window')withopenSideWindowAlertList()in thebeforeEachhook is a positive change. It uses a utility function to abstract away the details of opening the side window alert list, making the test more readable and maintainable.frontend/src/features/VesselSidebar/actions/TrackRequest/PositionsTable.tsx (7)
- 18-18: Adding an import for
VesselPositionfrom 'domain/entities/vessel/types' is a good practice. It ensures that the components use a specific, well-defined type for vessel positions, enhancing type safety and code readability.- 81-81: Modifying
SpeedCellPropsto extendCellProps<VesselPosition>is a positive change. It ensures that the props passed to theSpeedCellcomponent are correctly typed, enhancing type safety and code clarity.- 108-108: The updated null check in
SpeedCellto handlerowDatabeing potentially null or undefined is a good practice. It ensures the component behaves correctly in all cases, preventing runtime errors.- 113-113: Modifying
CourseCellPropsto extendCellProps<VesselPosition>improves type safety by ensuring that the props passed to theCourseCellcomponent are correctly typed. This change enhances code clarity and maintainability.- 140-140: The updated null check in
CourseCellto handlerowDatabeing potentially null or undefined is appropriate. It ensures the component behaves correctly under all circumstances, enhancing robustness.- 145-145: Modifying
DateTimeCellPropsto extendCellProps<VesselPosition>is a positive change for type safety. It ensures that the props passed to theDateTimeCellcomponent are correctly typed, improving code clarity.- 163-166: The updated null checks and handling of
rowDatainDateTimeCellto ensure correct formatting of the date string, including handling of 'Z' and '+' time zone indicators, are good practices. They enhance the robustness and correctness of the component's behavior.frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionCard.tsx (5)
- 62-66: The styled component
Wrappercorrectly applies conditional styling based on props. This is a good practice for dynamic styling based on component state or props.- 69-71: The usage of styled components for
Rowwith conditional props ($hasScroll,$isFirstRow,$marginTop) enhances readability and maintainability by encapsulating styling logic within the component itself.- 80-82: The conditional rendering of the
Flagcomponent based onbeaconMalfunction?.flagStateis correctly implemented. This ensures that the flag is only displayed when a flag state is available, preventing potential rendering issues or errors.- 106-107: The
EndOfMalfunctionstyled component correctly applies dynamic background and text color based on theendOfBeaconMalfunctionReason. This is a good example of using styled components to apply dynamic styles based on props.- 123-126: The styled component
Idis defined with static styles. It's important to ensure that these styles are consistent with the application's design system and that the removal of inline styles (if any were present before) has not affected the visual consistency of the component.Verification successful
Given the need to verify the consistency of the
Idcomponent's static styles with the application's design system, and the lack of direct information from the previously executed script, we will proceed to gather the necessary information by searching for the application's theme configuration or any related documentation. This will help us compare the specified styles (color: slateGray; font-size: 11px;) with the design system's guidelines.
The verification process has confirmed that the color
slateGrayused in theIdcomponent is indeed part of the application's design system, as evidenced by its definition in the theme file and usage across various components. This suggests that the component's color style is consistent with the design system. While there was no direct reference to the font size11pxin the design system documentation or theme files, the absence of such a reference does not necessarily indicate inconsistency. Specific font sizes may be used at the discretion of developers for particular UI elements, provided they align with the overall design principles. Therefore, the original review comment's concern about ensuring consistency with the design system appears to be addressed for the color, and there's no clear evidence to suggest that the font size11pxis inconsistent with the design system.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the static styles applied to the `Id` component are consistent with the design system. # This script is a placeholder for manual verification. echo "Manual verification required to ensure consistency with the design system."Length of output: 158
Script:
#!/bin/bash # Search for theme configuration or documentation related to the design system fd 'theme|styles|design-system' --exec grep -Hn 'slateGray\|11px' {}Length of output: 1424
frontend/src/features/map/draw/DrawModal.tsx (1)
- 168-169: Adding a
nameattribute to theStyledCoordinatesInputcomponent is a good practice for form elements, as it improves accessibility and ensures that form data can be correctly captured and processed.frontend/src/features/SideWindow/MissionList/FilterBar.tsx (3)
- 99-99: Changing the
isLightprop toisTransparenton theTextInputcomponent aligns with updated component APIs and can improve visual consistency by making the input field blend better with its background. Ensure that this change is consistently applied across all relevant components.- 113-113: The update from
isLighttoisTransparenton theFormikSelectcomponent is a good practice for maintaining consistency with component APIs and enhancing the visual integration of the select component with its surroundings.- 129-129: Applying the
isTransparentprop to theFormikMultiSelectcomponent instead ofisLightensures that the component's appearance is consistent with the updated styling guidelines and component APIs.frontend/src/features/Mission/components/MissionForm/MainForm/FormikMultiControlUnitPicker/ControlUnitSelect.tsx (1)
- 3-3: The removal of the
useNewWindowhook and related references inControlUnitSelect.tsxsimplifies the component by removing unnecessary dependencies. Ensure that this change does not negatively impact any modal or popup functionalities that might have relied on theuseNewWindowhook.frontend/src/features/SideWindow/Alert/AlertListAndReportingList/PendingAlertRow.tsx (1)
- 215-215: The change in the height of list items from
15to42could significantly impact the visual layout and user experience. Please confirm that this adjustment aligns with the project's design guidelines and has been tested for accessibility and responsiveness.frontend/cypress/e2e/side_window/reporting_list.spec.ts (1)
- 132-132: Updating the selector from
.rs-picker-search-bar-inputto.rs-search-box-inputin the test scenario. Please confirm that this change accurately reflects updates to the UI components and that the new selector is stable and consistent across the application for test automation purposes.frontend/src/features/Backoffice/fleet_segments/NewFleetSegmentModal.tsx (3)
- 1-2: The addition of imports for
Fieldfrom@mtes-mct/monitor-uiandisCypressfrom@utils/isCypressis a good step towards improving code modularity and testability. Please ensure that these dependencies are properly managed and do not introduce any version compatibility issues.- 14-14: Introducing the
IS_CYPRESSconstant for conditional behavior in Cypress tests is a practical approach to handling test-specific scenarios. However, ensure that this does not lead to code paths that are only executed in tests and not in production, as this could mask potential issues.- 50-58: Modifying the
ModalWithCustomHeightcomponent with additional props for Cypress handling (keyboard={!IS_CYPRESS}) is a clever way to improve testability. Just make sure this approach is consistently applied across other components where similar issues might arise during testing.frontend/src/features/Mission/components/MissionForm/ActionList/Item.tsx (2)
- 63-63: The change from
'Navire inconnu'to' - Navire inconnu'for theLAND_CONTROLcase in thegetActionTitlecall improves text readability by ensuring consistency with other action types. This change correctly addresses the formatting issue mentioned in the PR objectives.- 69-69: The change from
'Navire inconnu'to' - Navire inconnu'for theSEA_CONTROLcase in thegetActionTitlecall also enhances readability and consistency across different action types. This adjustment aligns with the PR's goal of refining text formatting.frontend/src/features/SideWindow/Alert/AlertListAndReportingList/PendingAlertsList.tsx (1)
- 259-259: Adjusting the
heightproperty of list items from15to42in thelistItemStylefunction significantly impacts the visual representation of list items, potentially improving readability and spacing. However, ensure this change aligns with the design system and does not introduce layout issues in other parts of the application.Verification successful
Given the limitations of verifying alignment with design system guidelines without access to external resources, the next step is to assess whether the height adjustment to
listItemStylemight introduce layout issues elsewhere in the application. To do this, I will search the codebase for references tolistItemStyle.
The search results show that
listItemStyleis referenced and defined withinPendingAlertRow.tsxandPendingAlertsList.tsx, indicating that the style adjustments are localized to these components. This localization suggests that the height adjustment from15to42is intended to improve the visual representation and usability of list items specifically within the context of pending alerts, without implying broader layout issues across the application. Given this context, it appears that the change is focused on enhancing the presentation of pending alerts, aligning with potential usability or readability improvements within these components.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if the height adjustment aligns with the design system guidelines. # This script is a placeholder for manual verification. echo "Manual verification needed to ensure alignment with design system guidelines."Length of output: 164
Script:
#!/bin/bash # Search for references to `listItemStyle` to assess potential impact on other parts of the application. rg "listItemStyle"Length of output: 606
frontend/src/components/CustomGlobalStyle.ts (1)
- 1-448: The
CustomGlobalStyle.tsfile provides global CSS styles for the application. It's important to ensure that these styles do not conflict with local component styles and maintain consistency across the application. Given that no specific changes are highlighted, it's assumed that this file is provided for context.frontend/cypress/e2e/backoffice/fleet_segments.spec.ts (7)
- 17-20: The addition of a log statement to clarify the intention of the test segment is a good practice for readability and maintainability.
- 34-36: Adding a log statement before updating the impact factor field improves test readability. It's beneficial for distinguishing between different test sections.
- 54-55: The log statement added before updating the segment field helps in understanding the test flow and intentions, enhancing the test's readability.
- 82-83: The log statement added before updating the segment name field is a good practice, making the test easier to follow and understand.
- 111-112: Introducing a log statement before updating the gears field is beneficial for test clarity and maintainability.
- 174-175: The addition of a log statement before the deletion of a fleet segment enhances the test's readability and clarity.
- 197-198: Adding a log statement before creating a fleet segment is a good practice, improving the readability and maintainability of the test.
frontend/cypress/e2e/vessels/vessel_track.spec.ts (1)
- 155-155: Updating the test to target a different element using a more specific selector is a good improvement. It likely enhances the reliability of the test by ensuring interactions are more precise and reflective of user actions.
frontend/src/features/VesselList/VesselListFilters.tsx (2)
- 32-32: The modification of
tagPickerStyleto adjust margin and vertical alignment is noted. Ensure that these changes align with the design system and do not negatively impact the layout or user experience on different screen sizes.- 317-317: Adjusting the
margin-topvalue ofFilterDescto 10px. This change should be verified to ensure it maintains visual consistency across the application and does not introduce any unexpected layout shifts.frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionDetailsFollowUp.tsx (3)
- 240-242: The adjustment of the width for
AddCommentRowto 570px is noted. Ensure this width adjustment aligns with the design system and does not negatively impact responsiveness or the user experience on different screen sizes.- 286-286: The update to
commentsAndActionsStyleto dynamically adjustmaxHeightbased onsmallSizeandtextAreaHeightis a good practice for responsive design. Verify that these changes enhance the user experience without introducing overflow or clipping issues in various screen sizes and states.- 308-312: Refactoring the
Bodystyle to include flex properties improves layout flexibility and responsiveness. Ensure that this change integrates well with the rest of the application's layout and does not introduce any alignment or spacing issues.frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionDetails.tsx (3)
- 46-46: The update to include
nullin the type declaration forvesselStatusElementimproves type safety by accounting for the possibility that the element may not be found. This is a good practice for robust error handling.- 51-51: Similarly, updating the type declaration for
selectElementto includenullenhances type safety. This change ensures that the code properly handles cases where the element might not be present.- 361-364: Adding styling properties to
BeaconMalfunctionDetailsWrapperis noted. Ensure that these styling adjustments align with the design system and do not negatively impact the layout or user experience on different screen sizes.frontend/src/features/SideWindow/BeaconMalfunctionBoard/index.tsx (2)
- 14-14: The import of
LegacyRsuiteComponentsWrapperfromui/LegacyRsuiteComponentsWrapperis correctly implemented. This encapsulation of legacy component styles in a separate styled component is a good practice for maintaining separation of concerns and facilitating future migrations or style refactors.- 384-388: The addition of styling for
.rs-picker-toggle-wrapperto adjust its height and margin is a good example of fine-tuning third-party component styles to fit the application's design requirements. However, ensure that such customizations are documented, especially if they are specific workarounds for issues encountered during the migration. This will help future developers understand the context and necessity of these adjustments.frontend/src/features/Reporting/components/ReportingList/index.tsx (2)
- 256-256: The addition of
white-space: nowrap;to theUnderCharterstyled component ensures that the text within this component does not wrap onto multiple lines. This change is beneficial for maintaining the layout consistency of the table.- 300-300: Adjusting the widths of various columns in the table aims to better accommodate the content within those columns. However, it's important to consider the feedback from previous reviews regarding the
widthproperty and screen resolutions. Ensure that these width adjustments do not negatively impact the usability of the table on devices with different screen sizes. Testing across various devices or using responsive units like percentages orvwmight help address these concerns.Also applies to: 304-304, 314-314, 318-318, 328-328, 332-332, 336-336, 342-342, 347-347
frontend/cypress/e2e/vessel_sidebar/logbook.spec.ts (1)
- 223-223: Updating the selectors from
#tripNumberto#tripNumber-describein the Cypress E2E tests improves the clarity and consistency of the test cases. This change likely aligns the selectors more closely with the updated DOM structure or naming conventions in the application, ensuring that the tests remain reliable and maintainable.Also applies to: 227-227, 230-230, 233-233, 236-236, 239-239, 252-252, 258-258
frontend/cypress/e2e/backoffice/new_regulation.spec.ts (2)
- 209-209: The use of
.forceClick()instead of a direct click action might indicate that the element is not interactable in a standard way, possibly due to being obscured or not visible. While this approach can be necessary in some testing scenarios, it's important to ensure that it doesn't mask underlying issues with the UI that could affect end-users.Consider verifying if the UI element's interaction issue can be resolved in a way that reflects the user's actual experience more closely.
- 268-271: The update from
.rs-picker-search-bar-inputto.rs-search-box-inputfor search box selectors is a necessary change to maintain compatibility with updated UI elements. This change ensures that the Cypress tests continue to interact with the UI as intended after the migration.Ensure that all instances where search boxes are used in the Cypress tests have been updated to reflect this change for consistency.
frontend/cypress/e2e/backoffice/update_regulation.spec.ts (4)
- 34-34: The change from
.rs-picker-toggle-valueto.rs-picker-date inputfor date input validation reflects an update to the UI component structure. This change is crucial for ensuring that the Cypress tests accurately interact with the date picker elements post-migration.It's important to verify that all date picker interactions in the Cypress tests have been updated to use the new selector to maintain consistency and test reliability.
- 115-118: The update from
.rs-picker-search-bar-inputto.rs-search-box-inputfor search box selectors is consistent with the changes observed in thenew_regulation.spec.tsfile. This adjustment ensures that the tests remain aligned with the updated UI components, allowing for accurate interaction with search boxes.As with the previous file, ensure that all instances where search boxes are used in the Cypress tests have been updated to reflect this change for consistency.
- 127-131: The repeated use of
.rs-search-box-inputto interact with search boxes in different contexts within the same test case demonstrates a consistent approach to adapting the tests to the updated UI components. This consistency is key to maintaining the reliability and readability of the test suite.Continue to apply this consistent approach to all relevant test cases to ensure comprehensive coverage and maintainability of the test suite.
- 317-317: The use of
.rs-picker-date inputfor date input validation in this context is consistent with the changes made in other test cases. This consistency across the test suite is crucial for ensuring that the tests accurately reflect the updated UI components and their interactions.Maintaining this consistency across all test cases involving date picker interactions is essential for the reliability and maintainability of the test suite.
frontend/src/features/VesselList/index.tsx (1)
- 384-457: The introduction of the
LegacyRsuiteComponentsWrapperaround the content of theVesselListcomponent is a significant structural change. Ensure this wrapper integrates seamlessly with the existing codebase, maintaining the component's functionality, styling, and performance. Additionally, verify that this change does not introduce any regressions or unexpected behavior, especially in terms of accessibility and responsiveness.frontend/cypress/e2e/side_window/mission_form/land_control.spec.ts (1)
- 40-40: The CSS selector for the search input field has been updated from
.rs-picker-search-bar-inputto.rs-search-box-input. This change is likely due to UI updates in themonitor-uiversion 13 migration. Ensure that this new selector is consistently used across all relevant tests to maintain uniformity and avoid potential issues with element targeting in future UI updates.frontend/cypress/e2e/side_window/beacon_malfunction/board.spec.ts (9)
- 3-3: The addition of
openSideWindowBeaconMalfunctionBoardfrom './utils' is a good practice as it abstracts the navigation logic into a utility function, making the test more readable and maintainable.- 9-9: Replacing direct navigation and click commands with
openSideWindowBeaconMalfunctionBoard()is an excellent improvement. It encapsulates the steps needed to reach the beacon malfunction board, enhancing test readability and potentially reducing duplication across tests.- 362-362: Using
.forceClick()instead of.click()can be necessary in certain testing scenarios where elements are not interactable due to overlays or other CSS issues. However, it's important to ensure this change doesn't mask underlying UI issues that could affect end-users. Please verify that the use of.forceClick()is indeed necessary and not a workaround for a UI bug.- 386-386: The change to use
cy.getDataCy('side-window-beacon-malfunctions-detail').contains('Envoyer un message').click()improves the selector specificity and readability. It's a good practice to use data attributes for test selectors to reduce the likelihood of tests breaking due to changes in the UI structure or styling.- 390-390: The use of
cy.getDataCyfor selecting elements is consistent with best practices for Cypress tests, making the tests more robust to changes in the application's UI. This change enhances the maintainability of the test suite.- 408-408: Maintaining consistency in using
cy.getDataCyfor element selection is beneficial for the reasons previously mentioned. It's good to see this approach being applied uniformly across the test suite.- 412-412: The continuation of using
cy.getDataCyfor selecting elements and interacting with the UI is noted and appreciated. This consistency in test implementation is crucial for maintainability and readability.- 460-460: Replacing direct text selectors with
cy.contains('Statut').click()is a simplification that improves the test's readability. However, it's essential to ensure that this change does not reduce the specificity of the selector to a point where it might accidentally target unintended elements in the future.- 474-474: The replacement of the clear filter action with a more specific selector
.rs-picker-clean.rs-btn-close.click()is a good practice. It directly targets the clear button, making the test more reliable and less prone to failure due to UI changes.frontend/cypress/e2e/side_window/mission_form/main_form.spec.ts (3)
- 25-25: The addition of a check for 'Nouvelle mission' text in an
h1element is a good practice for ensuring that the UI is in the expected state before proceeding with further actions in the test.- 81-81: Updating the text check from 'Nouvelle mission' to 'Mission Mer – Cultures marines 56' in an
h1element is a necessary change to align with the expected UI changes. This ensures that the test remains relevant and accurate.- 267-267: Adding a wait for an
@updateMissionrequest before further actions is a good practice to ensure that the test waits for necessary asynchronous operations to complete. This can help prevent flaky tests that fail due to timing issues.frontend/cypress/e2e/side_window/mission_form/sea_control.spec.ts (2)
- 98-99: The change from string to numeric values for "Maillage déclaré" and "Maillage mesuré" is a positive adjustment, ensuring that the test inputs align more closely with the expected data types in the application. This change enhances the test's accuracy and reliability.
- 799-799: The addition of a PAM control unit by setting the "Administration 1" field to
undefinedbefore filling "Unité 1" seems to be a part of a setup step for a test scenario. Ifcy.fillis being used to clear the field, ensure that this approach is consistent and well-documented. Additionally, verify that the test accurately reflects the application's behavior and that the setup steps are necessary for the scenario being tested.Consider verifying the necessity of clearing the "Administration 1" field before setting "Unité 1" and ensure that the test setup accurately reflects the intended user interactions.
| @@ -334,6 +337,7 @@ context('Side Window > Mission Form > Main Form', () => { | |||
| emitsAis: 'NOT_APPLICABLE', | |||
| emitsVms: 'YES', | |||
| externalReferenceNumber: null, | |||
| // TODO There is an issue here, it's loaded as "MENM", not edited and updated as "NAMO". | |||
There was a problem hiding this comment.
The addition of a comment regarding an issue with the 'facade' field loading as "MENM" instead of being updated as "NAMO" is useful for documenting known issues. However, it's important to track this in an issue tracker for resolution rather than leaving it as a comment in the code.
Would you like me to open a GitHub issue to track the resolution of this facade field loading issue?
Comment on lines
+366
to
+373
| background-color: ${p => p.theme.color.gainsboro}; | ||
| border-color: ${p => p.theme.color.gainsboro}; | ||
| display: inline-block; | ||
| margin-right: 35px; | ||
| width: 125px; | ||
| width: 160px; | ||
|
|
||
| .rs-picker-toggle-wrapper { | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
The styling updates for the .Field-Select class and its child .rs-picker-toggle-wrapper include changes to the background color, border color, and width. Ensure these changes align with the application's design system and consider accessibility implications, such as sufficient contrast ratios for readability.
| // cy.get('[data-cy="create-fleet-segment-incidental-species"] input').type('ANE', { force: true }) | ||
| cy.fill('Engins', ['DHS', 'FCN']) | ||
| cy.fill('Espèces ciblées', ['COD', 'SOL']) | ||
| // TODO Investigate why this is not working. |
There was a problem hiding this comment.
The TODO comment indicates an unresolved issue with the test. It's important to track this to ensure the test suite is comprehensive and all intended scenarios are covered.
Would you like me to investigate and propose a solution for the issue mentioned in the TODO comment?
Comment on lines
+1
to
+465
| .rs-picker-check-menu-group[role='listitem'] { | ||
| height: 48px !important; | ||
| } | ||
|
|
||
| .rs-picker-check-menu-items .rs-picker-check-menu-group:not(:first-child) { | ||
| padding-top: 0px !important; | ||
| } | ||
|
|
||
| .rs-table-cell-header .rs-table-cell-content { | ||
| padding: 10px 7px; | ||
| } | ||
|
|
||
| .rs-table-hover .rs-table-row:not(.rs-table-row-header):hover, | ||
| .rs-table-hover .rs-table-row:hover .rs-table-cell-group, | ||
| .rs-table-hover .rs-table-row:hover .rs-table-cell { | ||
| background: #d4dde7 !important; /* blueYonder 25 */ | ||
| } | ||
|
|
||
| .rs-table-cell, | ||
| .rs-table-cell-group, | ||
| .rs-table-row { | ||
| -webkit-transition: background-color 0.1s linear !important; | ||
| transition: background-color 0.1s linear !important; | ||
| } | ||
|
|
||
| .new-regulation-select-picker a { | ||
| padding: 8px 0px; | ||
| } | ||
|
|
||
| .rs-table-row-expanded { | ||
| height: 100px !important; | ||
| background: white !important; | ||
| } | ||
|
|
||
| .hover-border:hover { | ||
| border-bottom: 1px solid rgb(40, 47, 62); | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| .rs-btn-primary:hover { | ||
| color: inherit; | ||
| } | ||
|
|
||
| /***************************************************************************** | ||
| Fixes following mointor-ui v12 upgrade (including Rsuite breaking changes) | ||
| */ | ||
|
|
||
| .rs-picker { | ||
| border-radius: 0; | ||
| font-size: 13px; | ||
| width: 100%; | ||
|
|
||
| * { | ||
| font-size: 13px; | ||
| } | ||
|
|
||
| [role='combobox'] { | ||
| width: 100%; | ||
| } | ||
|
|
||
| .rs-picker-toggle-placeholder { | ||
| font-size: 11px; | ||
| } | ||
| } | ||
|
|
||
| .rs-checkbox { | ||
| .rs-checkbox-checker { | ||
| min-height: 0; | ||
|
|
||
| .rs-checkbox-wrapper { | ||
| .rs-checkbox-inner { | ||
| &:before { | ||
| background-color: ${p => p.theme.color.gainsboro} !important; | ||
| border: solid 2px ${p => p.theme.color.lightGray} !important; | ||
| border-radius: 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &.rs-checkbox-checked { | ||
| .rs-checkbox-wrapper { | ||
| .rs-checkbox-inner { | ||
| &:before { | ||
| background-color: ${p => p.theme.color.charcoal} !important; | ||
| border: solid 2px ${p => p.theme.color.charcoal} !important; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .rs-picker { | ||
| width: auto; | ||
| } | ||
|
|
||
| .rs-picker-popup { | ||
| z-index: 9999; | ||
| } | ||
| ` |
There was a problem hiding this comment.
The migration to version 13 of monitor-ui has led to significant styling adjustments within LegacyRsuiteComponentsWrapper.tsx. The changes are comprehensive, covering a wide range of UI components. Here are a few observations and recommendations:
-
Consistency and Maintainability: The use of styled-components for defining CSS is consistent and promotes maintainability. However, ensure that the theme colors (
p => p.theme.color.gainsboro,p => p.theme.color.lightGray, etc.) are defined in a centralized theme file. This approach facilitates easier updates and maintains consistency across the application. -
Performance Considerations: While styled-components are efficient, excessive use of nested selectors and dynamic properties (e.g.,
${p => p.theme.color.gainsboro}) can slightly impact performance. Consider minimizing deep nesting and evaluating the necessity of dynamic properties for static values. -
Best Practices: The use of
!importantis generally discouraged as it can make debugging and overriding styles more challenging. If possible, try to avoid it by using more specific selectors or reorganizing your CSS to avoid conflicts. -
Accessibility: Ensure that the color contrasts and interactive element sizes meet accessibility standards. This is crucial for users with visual impairments and for compliance with web accessibility guidelines.
-
Responsive Design: Verify that the styling adjustments are responsive and provide a consistent user experience across different screen sizes and devices. This might require additional media queries or adjustments to existing ones.
Overall, the changes appear well-structured and aimed at improving the application's look and feel post-migration. It's important to keep these considerations in mind for future styling adjustments.
Comment on lines
351
to
371
| margin-right: 5px; | ||
| margin-left: 5px; | ||
| } | ||
| .rs-toggle > input { | ||
| width: unset; | ||
|
|
||
| .rs-picker { | ||
| width: 180px !important; | ||
|
|
||
| > .rs-picker-toggle { | ||
| padding: 3px 8px 5px; | ||
|
|
||
| > .rs-stack { | ||
| > .rs-picker-toggle-indicator { | ||
| > svg { | ||
| top: 4px; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ` | ||
| const wrapperStyle: CSSProperties = { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [323-368]
The adjustments made to the .rs-picker and its nested components within Wrapper are specific and seem to be tailored to the application's design requirements post-migration. However, a few points to consider:
-
Specificity and Overriding Styles: The use of
!importantin line 356 to override the width of.rs-pickeris generally discouraged. If this is necessary due to the specificity of third-party component styles, ensure that this approach is used sparingly and documented for future reference. -
Maintainability: The nested selectors, while necessary for targeting specific elements of third-party components, can become hard to maintain. Consider documenting these overrides clearly or exploring alternative approaches, such as contributing to the third-party component for more customizable APIs.
-
Consistency with Theme: Ensure that the hardcoded colors (e.g.,
background: #1675e0 !important;) align with the application's theme. If these colors are part of the theme, use theme variables to maintain consistency and ease future theme adjustments.
Overall, the styling adjustments are understandable given the context of migrating to a new version of monitor-ui. Just ensure that these changes are maintainable and consistent with the overall design system.
Comment on lines
+204
to
+218
| const EndOfMalfunction = styled.div<{ $backgroundColor: string; $color: string }>` | ||
| background-color: ${p => p.$backgroundColor || 'unset'}; | ||
| border-radius: 11px; | ||
| color: ${p => p.$color || 'unset'}; | ||
| font-weight: 500; | ||
| height: 20px; | ||
| margin: 8px 12px 5px; | ||
| max-width: 185px; | ||
| overflow: hidden; | ||
| padding: 1px 10px; | ||
| text-align: left; | ||
| text-overflow: 'ellipsis'; | ||
| white-space: nowrap; | ||
| width: fit-content; | ||
| ` |
There was a problem hiding this comment.
The EndOfMalfunction styled component makes use of conditional styling for background-color and color based on props. However, the text-overflow property is set to a string value 'ellipsis' instead of the keyword ellipsis without quotes. This might be a typo that could affect the intended styling.
- text-overflow: 'ellipsis';
+ text-overflow: ellipsis;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.
Suggested change
| const EndOfMalfunction = styled.div<{ $backgroundColor: string; $color: string }>` | |
| background-color: ${p => p.$backgroundColor || 'unset'}; | |
| border-radius: 11px; | |
| color: ${p => p.$color || 'unset'}; | |
| font-weight: 500; | |
| height: 20px; | |
| margin: 8px 12px 5px; | |
| max-width: 185px; | |
| overflow: hidden; | |
| padding: 1px 10px; | |
| text-align: left; | |
| text-overflow: 'ellipsis'; | |
| white-space: nowrap; | |
| width: fit-content; | |
| ` | |
| const EndOfMalfunction = styled.div<{ $backgroundColor: string; $color: string }>` | |
| background-color: ${p => p.$backgroundColor || 'unset'}; | |
| border-radius: 11px; | |
| color: ${p => p.$color || 'unset'}; | |
| font-weight: 500; | |
| height: 20px; | |
| margin: 8px 12px 5px; | |
| max-width: 185px; | |
| overflow: hidden; | |
| padding: 1px 10px; | |
| text-align: left; | |
| text-overflow: ellipsis; | |
| white-space: nowrap; | |
| width: fit-content; | |
| ` |
Comment on lines
+14
to
+15
| // Hack to close the date picker popup | ||
| cy.get('h4').contains('Exporter les ACT-REP').click().wait(250) |
There was a problem hiding this comment.
Using a hack to close the date picker popup by clicking on an 'h4' element might lead to flaky tests if the UI changes. Consider exploring more robust solutions, such as directly interacting with the date picker's close functionality or improving the date picker's testability.
| @@ -17,7 +17,7 @@ export function CellUsingVesselProperty({ vesselProperty, ...props }: CellUsingV | |||
| return <Cell {...props}>{rowData => rowData.vesselProperties[vesselProperty]}</Cell> | |||
| } | |||
|
|
|||
| type CheckedCellProps = CellProps & { | |||
| type CheckedCellProps = CellProps<any> & { | |||
There was a problem hiding this comment.
The use of the generic parameter <any> in CheckedCellProps and CellWithTitle provides flexibility but can lead to a loss of type safety. Consider using more specific types or generics with constraints to ensure type safety.
Comment on lines
+14
to
+25
| const Value = styled.div` | ||
| align-items: center; | ||
| display: flex; | ||
| height: 22px; | ||
| ` | ||
| const valueStyle = { | ||
| color: '#282F3E' | ||
| } | ||
|
|
||
| const Text = styled.span`` | ||
| const Text = styled.span` | ||
| line-height: 1; | ||
| ` |
There was a problem hiding this comment.
The updates to the Value and Text styled components enhance the visual presentation of the component. Ensure that the valueStyle and textStyle objects are used appropriately or removed if they are no longer needed.
- const valueStyle = {
- color: '#282F3E'
- }
-
- const textStyle = {
- marginLeft: 5
- }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.
Suggested change
| const Value = styled.div` | |
| align-items: center; | |
| display: flex; | |
| height: 22px; | |
| ` | |
| const valueStyle = { | |
| color: '#282F3E' | |
| } | |
| const Text = styled.span`` | |
| const Text = styled.span` | |
| line-height: 1; | |
| ` | |
| const Value = styled.div` | |
| align-items: center; | |
| display: flex; | |
| height: 22px; | |
| ` | |
| const Text = styled.span` | |
| line-height: 1; | |
| ` |
Comment on lines
+1
to
+19
| import { SideWindowMenuLabel } from '../../../../src/domain/entities/sideWindow/constants' | ||
|
|
||
| export const openSideWindowAlertList = () => { | ||
| cy.viewport(1920, 1080) | ||
|
|
||
| cy.visit('/side_window') | ||
|
|
||
| cy.wait(500) | ||
|
|
||
| if (document.querySelector('[data-cy="first-loader"]')) { | ||
| cy.getDataCy('first-loader').should('not.be.visible') | ||
| } | ||
| cy.clickButton(SideWindowMenuLabel.ALERT_LIST_AND_REPORTING_LIST) | ||
| if (document.querySelector('[data-cy="first-loader"]')) { | ||
| cy.getDataCy('first-loader').should('not.be.visible') | ||
| } | ||
|
|
||
| cy.wait(500) | ||
| } |
There was a problem hiding this comment.
The openSideWindowAlertList function is correctly implemented for its intended purpose. Consider replacing cy.wait(500) with more deterministic wait conditions where possible to improve test reliability. Acknowledge that in some scenarios, a brief wait may be necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Linked issues
Summary by CodeRabbit
New Features
Enhancements
LegacyRsuiteComponentsWrapperfor consistent styling.Bug Fixes
Refactor
useNewWindowhook across several components.Style
Tests
Documentation