Feat/mn 569/add damage part select #829
Conversation
apps/demo-app/src/local-config.json
Outdated
| @@ -2,7 +2,7 @@ | |||
| "id": "demo-app-dev", | |||
| "description": "Config for the dev Demo App.", | |||
| "allowSkipRetake": true, | |||
| "enableAddDamage": true, | |||
| "addDamage": "two_shot", | |||
There was a problem hiding this comment.
switch to part_select by default on all apps
apps/drive-app/src/local-config.json
Outdated
| @@ -2,7 +2,7 @@ | |||
| "id": "drive-app-preview", | |||
| "description": "Config for the preview Drive App.", | |||
| "allowSkipRetake": true, | |||
| "enableAddDamage": false, | |||
| "addDamage": "two_shot", | |||
There was a problem hiding this comment.
switch to part_select by default on all apps
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "allowSkipRetake": true, | |||
| "enableAddDamage": false, | |||
| "addDamage": "two_shot", | |||
There was a problem hiding this comment.
switch to part_select by default on all apps
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "allowSkipRetake": true, | |||
| "enableAddDamage": true, | |||
| "addDamage": "two_shot", | |||
There was a problem hiding this comment.
switch to part_select by default on all apps
| '@monkvision/eslint-config-base', | ||
| 'plugin:@typescript-eslint/recommended', | ||
| 'plugin:import/typescript', | ||
| "@monkvision/eslint-config-base", |
There was a problem hiding this comment.
Your IDE probably modified the punctuation on this file by mistake. Go back to single quotes on the file.
| if (vehicleType === VehicleType.SUV) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| vehicleType = VehicleType.CUV; | ||
| } |
There was a problem hiding this comment.
I prefer this :
cons type = vehicleType === VehicleType.SUV ? VehicleType.CUV : vehicleType;over this :
if (vehicleType === VehicleType.SUV) {
// eslint-disable-next-line no-param-reassign
vehicleType = VehicleType.CUV;
}There was a problem hiding this comment.
This would be string forward and don't need to create a new variable. because vehicleType is good variable name.
| @@ -164,7 +164,7 @@ describe('InspectionGalleryItemCard component', () => { | |||
| const { unmount } = render(<InspectionGalleryItemCard {...props} />); | |||
|
|
|||
| expect(useInspectionGalleryItemStatusIconName).toHaveBeenCalled(); | |||
| expectPropsOnChildMock(Icon, { icon: useInspectionGalleryItemStatusIconName({} as any) }); | |||
| expectPropsOnChildMock(Icon, { icon: useInspectionGalleryItemStatusIconName({} as any)! }); | |||
There was a problem hiding this comment.
Even in tests, I really don't like using !, try to find a better solution
There was a problem hiding this comment.
Instead of this, we need to specify the exact type using the 'as' keyword to inform TypeScript about a different type But that also have it downsights.
| { allow: ["arrowFunctions"] }, | ||
| ], | ||
| "@typescript-eslint/no-empty-interface": OFF, | ||
| "@typescript-eslint/no-non-null-assertion": OFF, |
There was a problem hiding this comment.
remove this rule : it should stay enabled
There was a problem hiding this comment.
i have few clarification on this.
| @@ -129,7 +130,7 @@ export function PhotoCapture({ | |||
| customComplianceThresholdsPerSight, | |||
| useLiveCompliance = false, | |||
| allowSkipRetake = false, | |||
| enableAddDamage = true, | |||
| addDamage = AddDamage.TWO_SHOT, | |||
There was a problem hiding this comment.
Default should be part select
| /** | ||
| * The current state of the Add Damage mode of part selection. | ||
| */ | ||
| addDamagePartSelectState: 'part-select' | 'image-capture'; |
There was a problem hiding this comment.
Use an enum instead of magic strings for 'part-select' and 'image-capture'
There was a problem hiding this comment.
this change had been removed.
|
|
||
| /** | ||
| * Components implementing the main buttons of the PhotoCapture Camera HUD. This component implements 3 buttons : | ||
| * - A take picture button | ||
| * - A gallery button | ||
| * - A close button (only displayed if the `onClose` callback is defined) | ||
| * - A action button (only displayed if the `onAction` callback is defined) |
There was a problem hiding this comment.
...An action button...
There was a problem hiding this comment.
this change had been removed. Previous iteration required to keep the approve aka validate key on the right side above capture icon. where commonly close icon is present.
| > & { | ||
| onCancel: PhotoCaptureHUDElementsProps['onCancelAddDamage']; | ||
| partSelectState: PhotoCaptureHUDElementsProps['addDamagePartSelectState']; | ||
| }; |
There was a problem hiding this comment.
blank line under this
| import { PhotoCaptureHUDElementsProps } from '../PhotoCaptureHUDElements/PhotoCaptureHUDElements.model'; | ||
| import { usePhotoCaptureHUDElementsAddPartSelectShotStyle } from './usePhotoCaptureHUDElementsAddPartSelectShotStyle'; | ||
|
|
||
| type PhotoCaptureHUDElementsAddPartSelectShotProps = Pick< |
| const [selectedParts, setSelectedParts] = useState<VehiclePart[]>([]); | ||
| const { i18n, t } = useTranslation(); | ||
| const style = usePhotoCaptureHUDElementsAddPartSelectShotStyle(); | ||
| useEffect(() => onAddDamageParts(selectedParts), [selectedParts]); |
There was a problem hiding this comment.
blank lines above and under this hook
There was a problem hiding this comment.
this effect is removed.
| if (partSelectState === 'image-capture') { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
blank lines above and under this if statement
There was a problem hiding this comment.
this is removed
| > | ||
| {t('photo.hud.addDamage.selectParts')} | ||
| </div> | ||
| <VehiclePartSelection vehicleType={vehicleType!} onPartsSelected={setSelectedParts} /> |
There was a problem hiding this comment.
Do not use ! : find another solution
| | 'wrapper', | ||
| CSSProperties | ||
| > { | ||
| const { palette } = useMonkTheme(); |
There was a problem hiding this comment.
blank line under this
| @@ -60,7 +60,7 @@ export function PhotoCaptureHUDTutorial({ | |||
| sightId={sightId} | |||
| sightGuidelines={sightGuidelines} | |||
| enableSightGuidelines={currentTutorialStep === TutorialSteps.GUIDELINE} | |||
| enableAddDamage={true} | |||
| addDamage={AddDamage.TWO_SHOT} | |||
There was a problem hiding this comment.
Instead of passing AddDamage.TWO_SHOT to the addDamage prop, pass the real prop obtained from the PhotoCapture component
| tasks: tasksBySight?.[sightState.selectedSight.id] ?? sightState.selectedSight.tasks, | ||
| }); | ||
| break; | ||
| case PhotoCaptureMode.ADD_DAMAGE_PART_SELECT: |
There was a problem hiding this comment.
move this switch case above the SIGHT case to make it next to the other add damage cases
| @@ -17,7 +17,8 @@ | |||
| "damagedPartCounter": "1 / 2 • Damaged part", | |||
| "closeupPictureCounter": "2 / 2 • Closeup Picture", | |||
| "infoBtn": "Aim the target at the damaged part then tap the shutter button", | |||
| "infoCloseup": "Take a closeup picture of the damage" | |||
| "infoCloseup": "Take a closeup picture of the damage", | |||
| "selectParts": "Select the parts where you damage is" | |||
There was a problem hiding this comment.
...where the damage is...
| @@ -17,7 +17,8 @@ | |||
| "damagedPartCounter": "1 / 2 • Pièce endommagée", | |||
| "closeupPictureCounter": "2 / 2 • Dégât en gros plan", | |||
| "infoBtn": "Placer le viseur sur la pièce endommagée puis enclencher le bouton capture de la photo", | |||
| "infoCloseup": "Prendre une photo en gros plan du dégât" | |||
| "infoCloseup": "Prendre une photo en gros plan du dégât", | |||
There was a problem hiding this comment.
"Veuillez prendre une photo en gros plan du dégât"
| @@ -17,7 +17,8 @@ | |||
| "damagedPartCounter": "1 / 2 • Pièce endommagée", | |||
| "closeupPictureCounter": "2 / 2 • Dégât en gros plan", | |||
| "infoBtn": "Placer le viseur sur la pièce endommagée puis enclencher le bouton capture de la photo", | |||
| "infoCloseup": "Prendre une photo en gros plan du dégât" | |||
| "infoCloseup": "Prendre une photo en gros plan du dégât", | |||
| "selectParts": "Sélectionnez les pièces où vous endommagez" | |||
There was a problem hiding this comment.
Veuillez sélectionner les pièces endommagées
| en: `Damage on ${partsTranslation.map((part) => part.en).join(', ')}`, | ||
| fr: `Dommages sur ${partsTranslation.map((part) => part.en).join(', ')}`, | ||
| de: `Schaden an ${partsTranslation.map((part) => part.en).join(', ')}`, | ||
| nl: `Schade aan ${partsTranslation.map((part) => part.en).join(', ')}`, |
There was a problem hiding this comment.
Change french label to "Dégât sur ..."
Revert "♻️ change close button to action button and add validate action" This reverts commit d79b825.
arunachalam-monk
force-pushed
the
feat/MN-569/add-damage-part-select
branch
from
1e17e79
to
6ed9e92
Compare
arunachalam-monk
force-pushed
the
feat/MN-569/add-damage-part-select
branch
from
95b4be4
to
e43a6f3
Compare
arunachalam-monk
force-pushed
the
feat/MN-569/add-damage-part-select
branch
from
e43a6f3
to
065142c
Compare
Overview
Jira Ticket Reference : MN-569
Checklist before requesting a review