catchup
This commit is contained in:
@@ -125,7 +125,7 @@ patch_targets_path(pattern) if {
|
||||
}
|
||||
|
||||
file_path_pattern := `\.(js|jsx|ts|tsx|mjs|cjs)$`
|
||||
ignore_pattern := `//\s*biome-ignore|//\s*@ts-ignore|//\s*@ts-expect-error|//\s*@ts-nocheck|//\s*eslint-disable|/\*\s*eslint-disable`
|
||||
ignore_pattern := `//\s*biome-ignore|//\s*@ts-ignore|//\s*@ts-expect-error|//\s*@ts-nocheck|//\s*eslint-disable|/\*\s*biome-ignore|/\*\s*@ts-ignore|/\*\s*@ts-expect-error|/\*\s*@ts-nocheck|/\*\s*eslint-disable`
|
||||
|
||||
# Block Write/Edit operations that introduce ignore directives
|
||||
|
||||
|
||||
182
.sisyphus/plans/fix-meeting-card-checkbox-visibility.md
Normal file
182
.sisyphus/plans/fix-meeting-card-checkbox-visibility.md
Normal file
@@ -0,0 +1,182 @@
|
||||
# Fix MeetingCard Checkbox Visibility
|
||||
|
||||
## Context
|
||||
|
||||
### Original Request
|
||||
The MeetingCard checkbox is always visible regardless of selection mode state. It should only appear when the user clicks the "Select" button to enter selection mode.
|
||||
|
||||
### Root Cause Analysis
|
||||
The current implementation uses CSS-based hiding (`w-0 overflow-hidden`) on a `<fieldset>` element, which doesn't work reliably because:
|
||||
1. `<fieldset>` elements have special browser rendering behavior with default styles
|
||||
2. The checkbox inside may have minimum dimensions that prevent collapsing
|
||||
3. `overflow-hidden` on fieldsets doesn't clip child content consistently across browsers
|
||||
|
||||
### Solution
|
||||
Replace CSS-based show/hide with **conditional rendering** - only render the checkbox container when `isSelectable` is `true`.
|
||||
|
||||
### Pre-Validation
|
||||
- `npm run type-check` passes (0 errors)
|
||||
- `npm run lint` passes (0 errors)
|
||||
- Codebase is clean - no pre-existing type issues to fix
|
||||
|
||||
---
|
||||
|
||||
## Work Objectives
|
||||
|
||||
### Core Objective
|
||||
Make the checkbox in MeetingCard only visible when selection mode is active.
|
||||
|
||||
### Concrete Deliverables
|
||||
- Updated `client/src/components/features/meetings/meeting-card.tsx`
|
||||
|
||||
### Definition of Done
|
||||
- [ ] Checkbox is NOT visible when `isSelectable` is `false`
|
||||
- [ ] Checkbox IS visible when `isSelectable` is `true`
|
||||
- [ ] Clicking checkbox toggles selection (does not navigate)
|
||||
- [ ] `npm run type-check && npm run lint` passes with 0 errors
|
||||
|
||||
### Must Have
|
||||
- Conditional rendering based on `isSelectable` prop
|
||||
- Preserved click event handling (prevent navigation)
|
||||
- Preserved accessibility (`aria-label`)
|
||||
|
||||
### Must NOT Have (Guardrails)
|
||||
- No CSS-based hide/show tricks (already proven unreliable)
|
||||
- No `@ts-ignore` or type suppressions
|
||||
- No changes to `Meetings.tsx` (the parent is correct)
|
||||
|
||||
---
|
||||
|
||||
## Verification Strategy
|
||||
|
||||
### Test Decision
|
||||
- **Infrastructure exists**: YES (Vitest)
|
||||
- **User wants tests**: Manual verification sufficient for this UI fix
|
||||
- **QA approach**: Manual visual verification in browser
|
||||
|
||||
---
|
||||
|
||||
## TODOs
|
||||
|
||||
- [ ] 1. Update MeetingCard checkbox to use conditional rendering
|
||||
|
||||
**What to do**:
|
||||
1. Open `client/src/components/features/meetings/meeting-card.tsx`
|
||||
2. Replace the `<fieldset>` wrapper with conditional rendering
|
||||
3. Change from (lines 44-71):
|
||||
```tsx
|
||||
<fieldset
|
||||
aria-label={`Select ${meeting.title}`}
|
||||
className={cn(
|
||||
'flex items-center justify-center shrink-0 transition-all duration-200 border-0 p-0 m-0',
|
||||
isSelectable ? 'w-10 pl-3' : 'w-0 overflow-hidden'
|
||||
)}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Checkbox
|
||||
checked={isSelected}
|
||||
disabled={meeting.state === 'recording'}
|
||||
onCheckedChange={(checked) => onSelect?.(meeting.id, checked as boolean)}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
aria-label={`Select meeting: ${meeting.title}`}
|
||||
/>
|
||||
</fieldset>
|
||||
```
|
||||
4. Change to:
|
||||
```tsx
|
||||
{isSelectable && (
|
||||
<div
|
||||
role="group"
|
||||
aria-label={`Select ${meeting.title}`}
|
||||
className="flex items-center justify-center shrink-0 w-10 pl-3"
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Checkbox
|
||||
checked={isSelected}
|
||||
disabled={meeting.state === 'recording'}
|
||||
onCheckedChange={(checked) => onSelect?.(meeting.id, checked as boolean)}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
aria-label={`Select meeting: ${meeting.title}`}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
5. Remove the `cn` import if no longer used elsewhere in the file
|
||||
- Check line 13: `import { cn } from '@/lib/utils';`
|
||||
- Search for other uses of `cn` in the file - if found at line 42 (`className={cn(...)}` on Card), keep the import
|
||||
|
||||
**Must NOT do**:
|
||||
- Do not use CSS to hide/show (we're moving away from this approach)
|
||||
- Do not change the `isSelectable` prop logic in the parent
|
||||
- Do not modify `Meetings.tsx`
|
||||
|
||||
**Parallelizable**: NO (single task)
|
||||
|
||||
**References**:
|
||||
- `client/src/components/features/meetings/meeting-card.tsx:44-71` - Current fieldset implementation to replace
|
||||
- `client/src/pages/Meetings.tsx:292` - Shows `isSelectable` prop is passed correctly: `isSelectable={isSelectionMode && meeting.state !== 'recording'}`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- [ ] `cd client && npm run type-check` -> 0 errors
|
||||
- [ ] `cd client && npm run lint` -> 0 errors/warnings
|
||||
- [ ] Manual verification in browser:
|
||||
1. Navigate to Meetings page (`/meetings` or `/projects/{id}/meetings`)
|
||||
2. Verify checkboxes are NOT visible by default
|
||||
3. Click "Select" button in the filter bar
|
||||
4. Verify checkboxes appear on each card (except recording meetings)
|
||||
5. Click a checkbox - verify it toggles selection (card does NOT navigate)
|
||||
6. Click "Select" button again to exit selection mode
|
||||
7. Verify checkboxes disappear
|
||||
|
||||
**Commit**: YES
|
||||
- Message: `fix(client): hide MeetingCard checkbox when not in selection mode`
|
||||
- Files: `client/src/components/features/meetings/meeting-card.tsx`
|
||||
- Pre-commit: `npm run type-check && npm run lint`
|
||||
|
||||
---
|
||||
|
||||
## Commit Strategy
|
||||
|
||||
| After Task | Message | Files | Verification |
|
||||
|------------|---------|-------|--------------|
|
||||
| 1 | `fix(client): hide MeetingCard checkbox when not in selection mode` | meeting-card.tsx | type-check + lint |
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria
|
||||
|
||||
### Verification Commands
|
||||
```bash
|
||||
cd client && npm run type-check # Expected: 0 errors
|
||||
cd client && npm run lint # Expected: 0 errors/warnings
|
||||
```
|
||||
|
||||
### Final Checklist
|
||||
- [ ] Checkbox only visible in selection mode
|
||||
- [ ] No type errors
|
||||
- [ ] No lint warnings
|
||||
- [ ] Click behavior preserved (no navigation on checkbox click)
|
||||
Reference in New Issue
Block a user