RC-T40 Fixes #26
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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 file contains hidden or 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 file contains hidden or 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 file contains hidden or 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 file contains hidden or 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 file contains hidden or 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 file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,14 +31,16 @@ const LANE_NAME_SUGGESTION_KEYS = [ | |
| 'lane_suggestion_operations', | ||
| ]; | ||
|
|
||
| /** Optional per-lane limits: 0/undefined = no limit. Work-time thresholds: 0/blank = that color disabled. */ | ||
| export interface LaneLimits { | ||
| minUnits?: number; | ||
| maxUnits?: number; | ||
| minUnitPersonnel?: number; | ||
| maxUnitPersonnel?: number; | ||
| minTimeInRole?: number; | ||
| maxTimeInRole?: number; | ||
| workTimeAmberMinutes?: number; | ||
| workTimeRedMinutes?: number; | ||
| } | ||
|
|
||
| interface AddLaneSheetProps { | ||
|
|
@@ -54,7 +56,16 @@ export const AddLaneSheet: React.FC<AddLaneSheetProps> = ({ isOpen, onClose, onS | |
| const [nodeType, setNodeType] = useState<CommandNodeType>(CommandNodeType.Division); | ||
| const [color, setColor] = useState<string | undefined>(undefined); | ||
| const [showLimits, setShowLimits] = useState(false); | ||
| const [limits, setLimits] = useState<Record<keyof LaneLimits, string>>({ | ||
| minUnits: '', | ||
| maxUnits: '', | ||
| minUnitPersonnel: '', | ||
| maxUnitPersonnel: '', | ||
| minTimeInRole: '', | ||
| maxTimeInRole: '', | ||
| workTimeAmberMinutes: '20', | ||
| workTimeRedMinutes: '40', | ||
|
There was a problem hiding this comment.
Magic number Kody rule violation: Replace magic numbers with named constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| }); | ||
|
|
||
| const setLimit = useCallback((key: keyof LaneLimits, value: string) => { | ||
| setLimits((current) => ({ ...current, [key]: value.replace(/[^0-9]/g, '') })); | ||
|
|
@@ -76,7 +87,7 @@ export const AddLaneSheet: React.FC<AddLaneSheetProps> = ({ isOpen, onClose, onS | |
| setNodeType(CommandNodeType.Division); | ||
| setColor(undefined); | ||
| setShowLimits(false); | ||
| setLimits({ minUnits: '', maxUnits: '', minUnitPersonnel: '', maxUnitPersonnel: '', minTimeInRole: '', maxTimeInRole: '', workTimeAmberMinutes: '20', workTimeRedMinutes: '40' }); | ||
| onClose(); | ||
| }, [name, nodeType, color, limits, onSave, onClose]); | ||
|
|
||
|
|
@@ -180,6 +191,27 @@ export const AddLaneSheet: React.FC<AddLaneSheetProps> = ({ isOpen, onClose, onS | |
| </VStack> | ||
| </HStack> | ||
| <Text className="text-xs text-gray-500 dark:text-gray-400">{t('command.limit_time_help')}</Text> | ||
| <HStack space="sm"> | ||
| <VStack className="flex-1" space="xs"> | ||
| <Text className="text-xs font-medium text-gray-600 dark:text-gray-300">{t('command.limit_worktime_amber')}</Text> | ||
| <Input size="sm" variant="outline"> | ||
| <InputField | ||
| keyboardType="number-pad" | ||
| placeholder={t('command.limit_none')} | ||
| value={limits.workTimeAmberMinutes} | ||
| onChangeText={(v) => setLimit('workTimeAmberMinutes', v)} | ||
|
There was a problem hiding this comment.
Inline arrow functions in JSX props create new function instances on every render, impacting performance. Move these handlers outside the render method or memoize them. Kody rule violation: Avoid using .bind() or arrow functions in JSX props Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| testID="limit-worktime-amber" | ||
| /> | ||
| </Input> | ||
| </VStack> | ||
| <VStack className="flex-1" space="xs"> | ||
| <Text className="text-xs font-medium text-gray-600 dark:text-gray-300">{t('command.limit_worktime_red')}</Text> | ||
| <Input size="sm" variant="outline"> | ||
| <InputField keyboardType="number-pad" placeholder={t('command.limit_none')} value={limits.workTimeRedMinutes} onChangeText={(v) => setLimit('workTimeRedMinutes', v)} testID="limit-worktime-red" /> | ||
| </Input> | ||
| </VStack> | ||
| </HStack> | ||
| <Text className="text-xs text-gray-500 dark:text-gray-400">{t('command.limit_worktime_help')}</Text> | ||
|
Comment on lines
+194
to
+214
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Files matching add-lane-sheet:"
fd -a 'add-lane-sheet\.tsx$' . || true
if [ -f src/components/command/add-lane-sheet.tsx ]; then
echo
echo "Outline:"
ast-grep outline src/components/command/add-lane-sheet.tsx || true
echo
echo "Relevant lines 160-230:"
sed -n '160,230p' src/components/command/add-lane-sheet.tsx | nl -ba -v160
echo
echo "Imports/labels pattern:"
rg -n "InputField|accessibilityLabel|accessibilityLabelledBy|limitWorktimeAmber|workTimeAmberMinutes|limit_worktime" src/components/command/add-lane-sheet.tsx
fiRepository: Resgrid/IC Length of output: 895 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Imports and relevant input/limit sections:"
sed -n '1,45p' src/components/command/add-lane-sheet.tsx
printf '\n---\n'
sed -n '170,230p' src/components/command/add-lane-sheet.tsx
echo
echo "Search for gluestack accessibilityLabel APIs nearby:"
rg -n "InputField|accessibilityLabel|accessibilityLabelledBy|limit_worktime|workTimeAmberMinutes|workTimeRedMinutes" src/components src/components/command/add-lane-sheet.tsx || trueRepository: Resgrid/IC Length of output: 25660 Add accessible labels to the work-time threshold inputs. The nearby [low_effort_and_highreward] 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| </VStack> | ||
| ) : null} | ||
| </VStack> | ||
|
|
||
This file contains hidden or 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 file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,17 +26,25 @@ | |
| /** Two-line clamp: numberOfLines leaks to the DOM through the styling pipeline on web, so use CSS line-clamp there. */ | ||
| const twoLine = isWeb ? {} : ({ numberOfLines: 2 } as const); | ||
|
|
||
| /** Default work-time light thresholds (Tablet Command-style crew fatigue): green under 20m, amber under 40m, red past that. Lanes can override; 0 disables that color. */ | ||
| export const DEFAULT_WORK_TIME_AMBER_MINUTES = 20; | ||
| export const DEFAULT_WORK_TIME_RED_MINUTES = 40; | ||
|
|
||
| export const workTimeColor = (minutes: number, amberAfterMinutes: number = DEFAULT_WORK_TIME_AMBER_MINUTES, redAfterMinutes: number = DEFAULT_WORK_TIME_RED_MINUTES) => | ||
| redAfterMinutes > 0 && minutes >= redAfterMinutes ? '#ef4444' : amberAfterMinutes > 0 && minutes >= amberAfterMinutes ? '#f59e0b' : '#22c55e'; | ||
|
There was a problem hiding this comment.
Hex color literals Kody rule violation: Centralize string constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| /** | ||
| * Elapsed minutes since a resource was assigned, ticking once a minute. When the lane sets a | ||
| * MaxTimeInRole, exceeding it turns the light red and flags the resource as rotation-due. | ||
| * Amber/red thresholds come from the lane (0 = that color disabled; unset = 20/40 defaults). | ||
| */ | ||
| export const WorkTimeLight: React.FC<{ assignedOn?: string | null; rotationAfterMinutes?: number; amberAfterMinutes?: number; redAfterMinutes?: number; testID?: string }> = ({ | ||
| assignedOn, | ||
| rotationAfterMinutes, | ||
| amberAfterMinutes = DEFAULT_WORK_TIME_AMBER_MINUTES, | ||
| redAfterMinutes = DEFAULT_WORK_TIME_RED_MINUTES, | ||
| testID, | ||
| }) => { | ||
| const { t } = useTranslation(); | ||
| const [nowMs, setNowMs] = useState(() => Date.now()); | ||
|
|
||
|
|
@@ -57,7 +65,7 @@ | |
|
|
||
| return ( | ||
| <HStack className="items-center" space="xs" testID={testID}> | ||
| <NativeView style={{ width: 8, height: 8, borderRadius: 4, backgroundColor: isRotationDue ? '#ef4444' : workTimeColor(minutes, amberAfterMinutes, redAfterMinutes) }} /> | ||
| <Text className={`text-xs tabular-nums ${isRotationDue ? 'font-semibold text-error-600 dark:text-error-400' : 'text-gray-500 dark:text-gray-400'}`}>{`${minutes}m`}</Text> | ||
| {isRotationDue ? ( | ||
| <Badge action="error" variant="solid" testID={testID ? `${testID}-rotation` : undefined}> | ||
|
|
@@ -91,6 +99,10 @@ | |
| name: string; | ||
| /** Lane MaxTimeInRole (minutes) — flags the resource rotation-due when exceeded. */ | ||
| rotationAfterMinutes?: number; | ||
| /** Lane work-time amber threshold (0 = amber disabled; unset = 20). */ | ||
| amberAfterMinutes?: number; | ||
| /** Lane work-time red threshold (0 = red disabled; unset = 40). */ | ||
| redAfterMinutes?: number; | ||
| isSelected: boolean; | ||
| onSelect: (assignmentId: string) => void; | ||
| onDragStart: (assignmentId: string) => void; | ||
|
|
@@ -107,12 +119,12 @@ | |
| height: number; | ||
| } | ||
|
|
||
| const DraggableResourceCard: React.FC<DraggableResourceCardProps> = React.memo(({ assignment, name, rotationAfterMinutes, amberAfterMinutes, redAfterMinutes, isSelected, onSelect, onDragStart, onDragEnd, onDrop, onView }) => { | ||
| const { t } = useTranslation(); | ||
| const translation = useRef(new Animated.ValueXY()).current; | ||
| const dragReadyRef = useRef(false); | ||
| const panActiveRef = useRef(false); | ||
| const longPressTriggeredRef = useRef(false); | ||
| const [isDragging, setIsDragging] = useState(false); | ||
|
|
||
| const resetDrag = useCallback(() => { | ||
|
|
@@ -203,7 +215,7 @@ | |
| {name} | ||
| </Text> | ||
| </HStack> | ||
| <WorkTimeLight assignedOn={assignment.AssignedOn} rotationAfterMinutes={rotationAfterMinutes} amberAfterMinutes={amberAfterMinutes} redAfterMinutes={redAfterMinutes} testID={`landscape-worktime-${assignment.ResourceAssignmentId}`} /> | ||
| <Pressable accessibilityLabel={t('command.view_details')} accessibilityRole="button" className="p-1" onPress={handleView} testID={`landscape-resource-view-${assignment.ResourceAssignmentId}`}> | ||
| <Eye className="text-gray-400" size={16} /> | ||
| </Pressable> | ||
|
|
@@ -398,6 +410,8 @@ | |
| key={assignment.ResourceAssignmentId} | ||
| assignment={assignment} | ||
| rotationAfterMinutes={node.MaxTimeInRole} | ||
| amberAfterMinutes={node.WorkTimeAmberMinutes} | ||
| redAfterMinutes={node.WorkTimeRedMinutes} | ||
| isSelected={selectedAssignmentId === assignment.ResourceAssignmentId} | ||
| name={resolveResourceName(assignment.ResourceKind, assignment.ResourceId)} | ||
| onDragEnd={() => setDraggingAssignmentId(null)} | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unmount the new render during cleanup.
The test destructures
unmounton Line 59 but never calls it after the assertion, allowing the rendered tree to leak into later tests.As per coding guidelines, tests must always call
unmount()during cleanup.Proposed fix
expect(onSave).toHaveBeenCalledWith('Rehab', CommandNodeType.Division, undefined, { workTimeAmberMinutes: 15 }); + unmount();📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Coding guidelines