From bf4507e68004cef8849e7cb5af124ec6318fd317 Mon Sep 17 00:00:00 2001 From: Frank Kopp Date: Mon, 1 Apr 2024 08:32:32 +0200 Subject: [PATCH] Refactor, comment and reorganize EFB Checklist code --- .../EFB/Checklists/ChecklistItemComponent.tsx | 35 ++++++----- .../src/EFB/Checklists/Checklists.tsx | 58 ++++++++++++------- .../src/EFB/Checklists/ChecklistsPage.tsx | 8 +-- .../src/EFB/Checklists/CompletionButton.tsx | 24 ++++---- .../src/systems/instruments/src/EFB/Efb.tsx | 4 +- .../src/checklists/ChecklistInterfaces.ts | 3 +- .../src/checklists/ChecklistProvider.ts | 40 ++++++++++--- 7 files changed, 111 insertions(+), 61 deletions(-) diff --git a/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistItemComponent.tsx b/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistItemComponent.tsx index f5b31e132466..9b94b982bc65 100644 --- a/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistItemComponent.tsx +++ b/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistItemComponent.tsx @@ -17,24 +17,18 @@ interface ChecklistItemComponentProps { export const ChecklistItemComponent = ({ item, index }: ChecklistItemComponentProps) => { const dispatch = useAppDispatch(); + const { selectedChecklistIndex, checklists } = useAppSelector((state) => state.trackingChecklists); + const [checklistShake, setChecklistShake] = useState(false); const [autoFillChecklists] = usePersistentNumberProperty('EFB_AUTOFILL_CHECKLISTS', 0); - const { selectedChecklistIndex, checklists } = useAppSelector((state) => state.trackingChecklists); + const isItemCompleted = checklists[selectedChecklistIndex].items[index]?.completed; const firstIncompleteIdx = checklists[selectedChecklistIndex].items.findIndex((item) => { - if (autoFillChecklists) { - return !item.completed && !item.hasCondition; - } + if (autoFillChecklists) return !item.completed && !item.hasCondition; return !item.completed; }); - const itemCheckedAfterIncomplete = checklists[selectedChecklistIndex].items - .slice(firstIncompleteIdx) - .some((item) => item.completed && (autoFillChecklists ? !item.hasCondition : true)); - - const itemImproperlyUnchecked = index === firstIncompleteIdx && itemCheckedAfterIncomplete; - // convenience variables to make the JSX more readable const isLine = item.type !== undefined && item.type === 'LINE'; const isListItem = item.type === undefined || item.type === 'ITEM'; @@ -42,6 +36,11 @@ export const ChecklistItemComponent = ({ item, index }: ChecklistItemComponentPr const isAnyItemType = isListItem || isSublistItem; const isSubListHeader = item.type !== undefined && item.type === 'SUBLISTHEADER'; + const itemCheckedAfterIncompleteItems = checklists[selectedChecklistIndex].items + .slice(firstIncompleteIdx) + .some((item) => item.completed && (autoFillChecklists ? !item.hasCondition : true)); + const itemImproperlyUnchecked = index === firstIncompleteIdx && itemCheckedAfterIncompleteItems; + let color = 'text-theme-text'; if (isItemCompleted && !isLine) { color = 'text-utility-green'; @@ -49,22 +48,25 @@ export const ChecklistItemComponent = ({ item, index }: ChecklistItemComponentPr color = 'text-utility-red'; } + // If the user interacts with an auto complete item 3 times in a row, show a toast message + // to point out that autofill is enabled and the item cannot be interacted with. const [autoItemTouches, setAutoItemTouches] = useState(0); - useEffect(() => { - if (autoItemTouches === 5) { - toast.info('You cannot interact with this item because you have enabled the autofill checklist option in the Realism settings page.'); + if (autoItemTouches === 3) { + toast.info('You cannot interact with this item because you have enabled the ' + + 'autofill checklist option in the Realism settings page.'); setAutoItemTouches(0); } }, [autoItemTouches]); - const relevantChecklistIndices = getRelevantChecklistIndices(); + const relevantChecklistIndices = getRelevantChecklistIndices(); // relevant for the current flight phase const firstRelevantUnmarkedIdx = checklists.findIndex((cl, clIndex) => relevantChecklistIndices.includes(clIndex) && !cl.markedCompleted); const autoCheckable = selectedChecklistIndex >= firstRelevantUnmarkedIdx && autoFillChecklists; const handleChecklistItemClick = () => { if (isLine) return; // lines are not clickable + // If the item is auto-checkable and the user tries to interact with it, shake the checklist if (item.condition && autoCheckable) { setAutoItemTouches((old) => old + 1); setChecklistShake(true); @@ -74,17 +76,22 @@ export const ChecklistItemComponent = ({ item, index }: ChecklistItemComponentPr return; } + // toggle completion of the item in the reducer state dispatch(setChecklistItemCompletion({ checklistIndex: selectedChecklistIndex, itemIndex: index, completionValue: !isItemCompleted, })); + // if the item was completed, uncomplete the checklist in the reducer state if (isItemCompleted) { dispatch(setChecklistCompletion({ checklistIndex: selectedChecklistIndex, completion: false })); } }; + // if the item definition has an action, use that when the item is incomplete + // otherwise, use the result string + // example: action="CHECK" result="CHECKED" let actionResultString = item.result; if (!isItemCompleted && item.action !== undefined) { actionResultString = item.action; diff --git a/fbw-common/src/systems/instruments/src/EFB/Checklists/Checklists.tsx b/fbw-common/src/systems/instruments/src/EFB/Checklists/Checklists.tsx index f2ef09237a24..0ece637cf031 100644 --- a/fbw-common/src/systems/instruments/src/EFB/Checklists/Checklists.tsx +++ b/fbw-common/src/systems/instruments/src/EFB/Checklists/Checklists.tsx @@ -1,7 +1,6 @@ // Copyright (c) 2023-2024 FlyByWire Simulations // SPDX-License-Identifier: GPL-3.0 -/* eslint-disable max-len */ import React, { useEffect, useState } from 'react'; import { usePersistentNumberProperty } from '@flybywiresim/fbw-sdk'; import { Link45deg } from 'react-bootstrap-icons'; @@ -16,6 +15,8 @@ import { } from '../Store/features/checklists'; import { RootState, store, useAppDispatch, useAppSelector } from '../Store/store'; +// ChecklistProvider is a singleton that reads the checklists from aircraft-specific json and +// provides it as a data structure const checklistReader = ChecklistProvider.getInstance(); /** @@ -60,15 +61,15 @@ export const getRelevantChecklistIndices = () => { * This is called every 1s from EFB.tsx and every time the selected checklist index changes. */ export const setAutomaticItemStates = (aircraftChecklists: ChecklistJsonDefinition[]) => { - if (aircraftChecklists.length === 0) return; + if (aircraftChecklists.length === 0) return; // in case the checklists are not loaded yet const checklists = (store.getState() as RootState).trackingChecklists.checklists; - // leave completed checklists alone - as otherwise they would be reset everytime an item becomes uncompleted - // iterate over all non-completed checklists and check all auto-checkable items checklists .forEach((cl, currentChecklistIdx) => { - if (cl.markedCompleted) return; // do not use filter as it would mess up the index sync between the two arrays + // leave completed checklists alone - as otherwise they would be reset everytime an item becomes uncompleted + // iterate over all non-completed checklists and check all auto-checkable items + if (cl.markedCompleted) return; // check all items in the current checklist if they are auto completed aircraftChecklists[currentChecklistIdx].items.forEach((clItem, itemIdx) => { @@ -77,6 +78,7 @@ export const setAutomaticItemStates = (aircraftChecklists: ChecklistJsonDefiniti // if the item is a line or subheader, mark it as completed as these do not have a relevant completion state if (clItem.type !== undefined && (clItem.type === 'LINE' || clItem.type === 'SUBLISTHEADER')) { isCompleted = true; + // if the item has a condition, check if it is fulfilled } else if (clItem.condition && clItem.condition.length > 0) { isCompleted = clItem.condition.every((c) => { let comp: string = c.comp; @@ -93,8 +95,8 @@ export const setAutomaticItemStates = (aircraftChecklists: ChecklistJsonDefiniti return false; } }); + // ignore items and subitems without a condition } else { - // ignore items without a condition return; } @@ -133,24 +135,24 @@ export const Checklists = () => { useEffect(() => { if (!autoFillChecklists) return; setAutomaticItemStates(aircraftChecklists); - }, [selectedChecklistIndex]); + }, [selectedChecklistIndex, autoFillChecklists]); const relevantChecklistIndices = getRelevantChecklistIndices(); - const firstRelevantUnmarkedIdx = checklists.findIndex((cl, clIndex) => relevantChecklistIndices.includes(clIndex) && !cl.markedCompleted); + const firstRelevantUnmarkedIdx = checklists.findIndex( + (cl, clIndex) => relevantChecklistIndices.includes(clIndex) && !cl.markedCompleted, + ); /** - * Handles the click event for a checklist item. - * - * @param {number} index - The index of the checklist item being clicked. - * @returns {void} + * @brief Handles the click event for a checklist item. + * @param index - The index of the checklist item being clicked. */ - const handleClick = (index: number): void => { + const handleClick = (index: number) => { dispatch(setSelectedChecklistIndex(index)); }; /** * @brief Get the css/tailwind class name for the checklist tab-button - * @param index + * @param index - The index of the checklist tab. */ const getTabClassName = (index: number) => { const isChecklistCompleted = areAllChecklistItemsCompleted(index); @@ -163,13 +165,21 @@ export const Checklists = () => { return 'bg-theme-highlight font-bold text-theme-body'; } if (isChecklistCompleted) { - return isMarkedCompleted ? 'bg-theme-body border-2 border-utility-green font-bold text-utility-green hover:text-theme-body hover:bg-utility-green' : 'bg-theme-body border-2 border-utility-amber font-bold text-utility-amber hover:text-theme-body hover:bg-utility-amber'; + return isMarkedCompleted ? 'bg-theme-body border-2 border-utility-green font-bold text-utility-green ' + + 'hover:text-theme-body hover:bg-utility-green' : 'bg-theme-body border-2 border-utility-amber ' + + 'font-bold text-utility-amber hover:text-theme-body hover:bg-utility-amber'; } return 'bg-theme-accent border-2 border-theme-accent font-bold text-theme-text hover:bg-theme-highlight hover:text-theme-body'; }; - const handleResetConfirmation = () => { - if (aircraftChecklists.length === 0) return; + /** + * @brief Function to handle the confirmation to reset all checklists. + * This function displays a confirmation modal with a warning message and a confirmation button. + * If the user confirms the reset, it will set the completion value for all checklist items and checklists + * to false. + */ + const handleResetAllConfirmation = () => { + if (aircraftChecklists.length === 0) return; // in case the checklists are not loaded yet showModal( { ); }; + /** + * @brief Handles the reset of a single checklist. + * + * This function sets the completion of each checklist item to false and the completion + * of the entire checklist to false. + */ const handleResetChecklist = () => { if (aircraftChecklists.length === 0) return; checklists[selectedChecklistIndex].items.forEach((_, itemIdx) => { @@ -208,8 +224,8 @@ export const Checklists = () => { dispatch(setChecklistCompletion({ checklistIndex: selectedChecklistIndex, completion: false })); }; - // aircraftChecklists are retrieved asynchronous there it is possible for aircraftChecklists to be empty. No point - // in rendering then. + // aircraftChecklists are retrieved asynchronous there it is possible for aircraftChecklists to be empty. + // No point in rendering in this case. if (aircraftChecklists.length === 0) { return ( <> @@ -242,7 +258,7 @@ export const Checklists = () => { @@ -256,7 +272,7 @@ export const Checklists = () => { - + ); diff --git a/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistsPage.tsx b/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistsPage.tsx index 8db852d5b326..ef7eaacfd170 100644 --- a/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistsPage.tsx +++ b/fbw-common/src/systems/instruments/src/EFB/Checklists/ChecklistsPage.tsx @@ -9,15 +9,15 @@ import { ChecklistItemComponent } from './ChecklistItemComponent'; import { CompletionButton } from './CompletionButton'; interface ChecklistPageProps { - acl: ChecklistJsonDefinition[]; + allChecklists: ChecklistJsonDefinition[]; } -export const ChecklistPage = ({ acl }: ChecklistPageProps) => { +export const ChecklistPage = ({ allChecklists }: ChecklistPageProps) => { const { selectedChecklistIndex } = useAppSelector((state) => state.trackingChecklists); return (
- {acl[selectedChecklistIndex].items.map((it, index) => ( + {allChecklists[selectedChecklistIndex].items.map((it, index) => ( { ))} - +
); }; diff --git a/fbw-common/src/systems/instruments/src/EFB/Checklists/CompletionButton.tsx b/fbw-common/src/systems/instruments/src/EFB/Checklists/CompletionButton.tsx index 95d2f8e7fca4..40cfe53cb292 100644 --- a/fbw-common/src/systems/instruments/src/EFB/Checklists/CompletionButton.tsx +++ b/fbw-common/src/systems/instruments/src/EFB/Checklists/CompletionButton.tsx @@ -14,22 +14,20 @@ import { } from '../Store/features/checklists'; interface CompletionButtonProps { - acl: ChecklistJsonDefinition[]; + allChecklists: ChecklistJsonDefinition[]; } -export const CompletionButton = ({ acl }: CompletionButtonProps) => { +export const CompletionButton = ({ allChecklists }: CompletionButtonProps) => { const dispatch = useAppDispatch(); const { selectedChecklistIndex, checklists } = useAppSelector((state) => state.trackingChecklists); + const [autoFillChecklists] = usePersistentNumberProperty('EFB_AUTOFILL_CHECKLISTS', 0); - const [completeItemVar, setCompleteItemVar] = useSimVar('L:A32NX_EFB_CHECKLIST_COMPLETE_ITEM', 'bool', 200); const firstIncompleteIdx = checklists[selectedChecklistIndex].items.findIndex((item, index) => { - const checklistItem = acl[selectedChecklistIndex].items[index]; + const checklistItem = allChecklists[selectedChecklistIndex].items[index]; // skip line items - if (checklistItem.type !== undefined && checklistItem.type === 'LINE') { - return false; - } + if (checklistItem.type !== undefined && checklistItem.type === 'LINE') return false; // Let's go ahead and skip checklist items that have a completion-determination function as those can't be manually checked. if (autoFillChecklists) { return !item.completed && !checklistItem.condition; @@ -37,16 +35,15 @@ export const CompletionButton = ({ acl }: CompletionButtonProps) => { return !item.completed; }); + // allows the completion button to be used via LVar - if the LVar is set to true, the button will be clicked, + // and the LVar will be reset to false. This can be used, for example, to trigger completion from a hardware button. + const [completeItemVar, setCompleteItemVar] = useSimVar('L:A32NX_EFB_CHECKLIST_COMPLETE_ITEM', 'bool', 200); useEffect(() => { setCompleteItemVar(false); }, []); - - // allows the completion button to be used via LVar - if the LVar is set to true, the button will be clicked, - // and the LVar will be reset to false useEffect(() => { if (completeItemVar) { setCompleteItemVar(false); - if (checklists[selectedChecklistIndex].markedCompleted && selectedChecklistIndex < checklists.length - 1) { dispatch(setSelectedChecklistIndex(selectedChecklistIndex + 1)); } else if (firstIncompleteIdx !== -1) { @@ -61,6 +58,8 @@ export const CompletionButton = ({ acl }: CompletionButtonProps) => { } }, [completeItemVar]); + // If the checklist is already marked as completed, show a button to proceed to the next checklist or + // a message if it's the last checklist. if (checklists[selectedChecklistIndex].markedCompleted) { if (selectedChecklistIndex < checklists.length - 1) { return ( @@ -87,6 +86,7 @@ export const CompletionButton = ({ acl }: CompletionButtonProps) => { ); } + // If there are incomplete items in the checklist, show a button to mark the first incomplete item as complete. if (firstIncompleteIdx !== -1) { return (
{ ); } + // If all items in the checklist are complete, show a button to mark the checklist as complete. if (areAllChecklistItemsCompleted(selectedChecklistIndex)) { return (
{ ); } + // If there are remaining autofill checklist items that have not yet been completed, show a message. return (
} A promise that resolves with an array of ChecklistJsonDefinition + * objects representing the checklists. + */ public async readChecklist(): Promise { if (this.checklists.length > 0) { return this.checklists; @@ -44,21 +55,36 @@ export class ChecklistProvider { // ============================================================================================= private constructor() { + // TODO: adapt to the new unified configuration (PR #8599) const aircraft = getAircraftType(); this.configFilename = `/VFS/${aircraft}_checklists.json5`; } + /** + * Process the checklist JSON5 data and issue a warning if the JSON5 data is invalid. + * + * @param {string} rawData - The raw JSON data to process. + */ private processChecklistJson(rawData: string) { try { const json = JSON5.parse(rawData); this.processChecklists(json); } catch (error) { - this.handleJsonParseError(error); + console.error(`Failed to parse ${this.configFilename} checklists as JSON5: `, error); } } + /** + * Processes the checklists from the given JSON object. + * + * The checklists are processed and added to the checklists-array. + * Invalid checklists are logged as warnings and ignored.. + * + * @param {any} json - The JSON object containing the checklists. + */ private processChecklists(json: any) { const checklists:ChecklistJsonDefinition[] = json.checklists; + // check each checklist's items for validity and add valid checklists to the checklist's array checklists.forEach((checklist, _) => { const checklistItems = []; const items:ChecklistItem[] = checklist.items; @@ -76,10 +102,6 @@ export class ChecklistProvider { }); } - private handleJsonParseError(error: Error) { - console.error(`Failed to parse ${this.configFilename} checklists as JSON5: `, error); - } - /** * Smoke testing a json checklist item for correct definition * *