Project: workspace-doctolib · Branch: HEAD · Mode: · Model: claude-opus-4-6 Stats: 1 user messages, 34 tool calls, tools used: Bash, Glob, Read. Conversation Turn 1 — User Review PR doctolib/treatment-plan#2935. The PR is titled "feat(fhir-2190)!: improve NHR document list filter panel UI" and contains UI improvements to the NHR document list filter panel in the tpf-national-health-records-ui package. IMPORTANT: Do NOT post any comments or review to GitHub. Only report findings in your response. PR Summary - Widen filter panel to 528px (from 400px) to match Figma spec - Use Title/L/Bold for the "Filters" header - Replace Tailwind border dividers with Oxygen Divider component - Remove per-filter separator lines; use p-16 padding on each filter wrapper - Apply customStyle={['w-full']} to BetaComboBox for Document Category dropdown - Use wrap={true} on RadioGroup to prevent flex spreading - Add formatNhrDateTime to nhrDateHelpers for locale-aware date+time display - Format lastAccessDate with formatNhrDateTime (formatting happens in factory, not component) - Add comprehensive unit tests for formatNhrDate and formatNhrDateTime - Refactor createNhrDocumentCategoryFilterConfig from positional params to single object param - Add optional placeholder prop to document category filter Changed files - packages/tpf-national-health-records-ui/.storybook/preview.tsx (+1/-1) - src/business/factories/filters/createNhrDateSelectionFilterConfig.test.tsx (+6/-3) - src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx (+2/-1) - src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx (+1/-1) - src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx (+13/-1) - src/components/NhrDateSelectionFilter/NhrDateSelectionFilter.tsx (+1/-1) - src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.stories.tsx (+1/-0) - src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx (+7/-0) - src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx (+7/-1) - src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.test.tsx (+18/-0) - src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.tsx (+1/-1) - src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.stories.tsx (+54/-127) - src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.test.tsx (+72/-0) - src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx (+31/-18) - src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx (+1/-1) - src/types.ts (+2/-0) - src/utils/nhrDateHelpers.test.ts (+133/-0) - src/utils/nhrDateHelpers.ts (+15/-0) Key architectural decisions to review 1. formatNhrDateTime formats the date in the factory (createNhrDateSelectionFilterConfig), not in the presentational component (NhrDateSelectionFilter). The component receives an already-formatted string. 2. NhrDocumentStatusFilter uses ?? (nullish coalescing) to choose between status.isSelected and value.includes(status.filterKey). 3. The storybook file was significantly simplified (127 lines removed, 54 added). Files to read The key source files are in /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/. Read as many as needed to do a thorough review. Focus especially on: - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.test.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/b …(truncated, 480 more chars) Turn 1 — Assistant I'll start by reading the project guidelines and the key changed files to perform a thorough review. Tools used: - Bash: cat CLAUDE.md 2>/dev/null; echo "---"; ls .claude/rules/ 2>/dev/null Tool results: → result (ok): # CLAUDE.md This is a full-stack healthcare treatment plan application (Kotlin/Spring Boot backend + React/TypeScript frontend) built as a modular monolith using the "hive" pattern with hexagonal architecture. It handles prescription creation, document management, and clinical workflows for the French and German markets. Refer to @AGENTS.md for complete project overview, architecture, tech stack, and development guidelines. Language Rule All Claude stack files (CLAUDE.md, AGENTS.m …(truncated, 243 more chars) Tools used: - Bash: git diff main...HEAD --name-only 2>/dev/null || echo "Not a git repo or no diff available" Tool results: → result (ok): .claude/rules/frontend/structure/package-structure.md .github/scripts/find-claudable-ticket.sh .github/scripts/guard-claudable-in-progress.sh .github/workflows/claude_doc_update.yml .github/workflows/frontend_module_federation_sync.yml .github/workflows/org_auto_request_claude.yml .github/workflows/releaseplease.yml .github/workflows/tpf-claude-dispatch.yml .node-version .release-please-manifest.json CODEOWNERS backend/prescription-electronic-fulfillments/prescription-electronic-fulfillments-dom …(truncated, 9969 more chars) Tools used: - Glob: .claude/rules// in /Users/USER/workspace/doctolib/treatment-plan Tool results: → result (ok): .claude/rules/backend/access-control.md .claude/rules/infrastructure/database.md .claude/rules/backend/architecture.md .claude/rules/backend/kotlin-conventions.md .claude/rules/backend/testing-patterns.md .claude/rules/frontend/i18n.md .claude/rules/frontend/storybook.md .claude/rules/infrastructure/api.md .claude/rules/modules/creating.md .claude/rules/testing/templates/component-test.tsx .claude/rules/testing/templates/controller-test.kt .claude/rules/testing/templates/msw-handler.ts .claude/r …(truncated, 1164 more chars) Now let me read the project rules and the key source files. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/general.md Tool results: → result (ok): 1 # Project Overview 2 3 This is a full-stack treatment plan application for healthcare, built with a Kotlin backend and React frontend. It's a monorepo containing multiple packages and modules for different aspects of treatment planning functionality. 4 5 ## Documentation Consultation 6 7 IMPORTANT: Before any new coding task (PLAN phase), you MUST: 8 9 - Search the TPF Confluence space (PTA) for business domain, architecture decisions, and existing patterns 10 - See AGENTS.md for k …(truncated, 4988 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/frontend/react/react.md Tool results: → result (ok): 1 --- 2 paths: 3 - "packages//.ts" 4 - "packages//.tsx" 5 - "packages//.css" 6 - "packages//.scss" 7 --- 8 9 # Frontend React/TypeScript Conventions 10 11 ## Code Style 12 13 - ESLint + Prettier configuration via @doctolib/upfront-dev 14 - Pre-commit hooks: Managed by Husky 15 16 ## Commands 17 18 - npm run lint - Run ESLint across all workspaces 19 - npm run lint:ci - Run ESLint for CI 20 - npm run test - Run tests across all workspaces 21 - npm run for …(truncated, 2076 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/frontend/react/test/testing.md Tool results: → result (ok): 1 --- 2 paths: 3 - "packages//.test.ts" 4 - "packages//.test.tsx" 5 - "packages//.spec.ts" 6 - "packages//.spec.tsx" 7 - "packages//vitest.config.ts" 8 - "packages//tests//" 9 --- 10 11 # Frontend Testing Conventions 12 13 ## Framework & Tools 14 15 - Test runner: Vitest 16 - Component testing: React Testing Library 17 - HTTP mocking: MSW (Mock Service Worker) 18 - User interactions: @testing-library/user-event 19 - Storybook tests: @story …(truncated, 3262 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/frontend/react/test/testing-patterns.md Tool results: → result (ok): 1 --- 2 paths: 3 - "packages//.test.ts" 4 - "packages//.test.tsx" 5 - "packages//.spec.ts" 6 - "packages//.spec.tsx" 7 - "packages//vitest.config.ts" 8 - "packages//tests//" 9 --- 10 11 # Frontend Testing Patterns 12 13 Advanced testing patterns for the TPF frontend. For core conventions (MSW, fixtures, Vitest config), see testing.md. 14 15 ## Component Testing 16 17 ### Basic Pattern 18 19 20 import { describe, it, expect, vi, afterEach } from …(truncated, 14787 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/frontend/react/oxygen-design-system.md Tool results: → result (ok): 1 --- 2 paths: 3 - "packages//.tsx" 4 --- 5 6 # Oxygen Design System Usage 7 8 ## Component Imports 9 10 Import from @doctolib/design-system submodules — prefer v2 components: 11 12 typescript 13 import { Button } from '@doctolib/design-system/Button/v2' 14 import { IconButton } from '@doctolib/design-system/IconButton/v2' 15 import { Divider } from '@doctolib/design-system/Divider' 16 import { Paragraph } from '@doctolib/design-system/Paragraph' 17 import { SpinnerLoader } from '@ …(truncated, 1738 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/.claude/rules/frontend/storybook.md Tool results: → result (ok): 1 --- 2 paths: 3 - "packages//.stories.ts" 4 - "packages//.stories.tsx" 5 - "packages//.storybook//" 6 --- 7 8 # Storybook Conventions 9 10 ## Architecture: Composed Storybook 11 12 - Hub: packages/tpf-storybook aggregates all package Storybooks via refs 13 - Packages: Each package has its own .storybook/ config and stories 14 - Main Storybook: https://doctolib.github.io/treatment-plan/ 15 - PR Previews: Auto-deployed at https://doctolib.github.io/treat …(truncated, 2459 more chars) Now let me read the key source files for the PR. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx Tool results: → result (ok): 1 import { ArrowRotateLeftRegularIcon } from '@doctolib/design-system-icons/ArrowRotateLeftRegularIcon' 2 import { Button } from '@doctolib/design-system/Button/v2' 3 import { Divider } from '@doctolib/design-system/Divider' 4 import { IconButton } from '@doctolib/design-system/IconButton/v2' 5 import { Paragraph } from '@doctolib/design-system/Paragraph' 6 import { useEffect, useState } from 'react' 7 8 import type { FilterConfig } from '../../types' 9 10 export type NhrDocumentListFilterPane …(truncated, 4194 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.ts Tool results: → result (ok): 1 const FALLBACK = '–' 2 3 export const formatNhrDate = (isoDate: string | null | undefined): string => { 4 if (!isoDate) return FALLBACK 5 const date = new Date(isoDate) 6 return Number.isNaN(date.getTime()) ? FALLBACK : date.toLocaleDateString() 7 } 8 9 export const formatNhrDateTime = (isoDate: string | null | undefined): string => { 10 if (!isoDate) return FALLBACK 11 const date = new Date(isoDate) 12 return Number.isNaN(date.getTime()) 13 ? FALLBACK 14 : date.toLocaleS …(truncated, 207 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.test.ts Tool results: → result (ok): 1 import { describe, expect, it } from 'vitest' 2 3 import { formatNhrDate, formatNhrDateTime } from './nhrDateHelpers' 4 5 const FALLBACK = '–' 6 7 describe('formatNhrDate', () => { 8 describe('when given a nullish or empty value', () => { 9 it('should return fallback for null', () => { 10 expect(formatNhrDate(null)).toBe(FALLBACK) 11 }) 12 13 it('should return fallback for undefined', () => { 14 expect(formatNhrDate(undefined)).toBe(FALLBACK) 15 }) 16 17 …(truncated, 4300 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx Tool results: → result (ok): 1 import { formatNhrDateTime } from '#utils/nhrDateHelpers' 2 import type { Option } from '@doctolib/design-system/Option' 3 4 import { 5 type DateMode, 6 NhrDateSelectionFilter, 7 } from '../../../components/NhrDateSelectionFilter/NhrDateSelectionFilter' 8 import type { FilterConfig } from '../../../types' 9 10 export const createNhrDateSelectionFilterConfig = ({ 11 label, 12 dateTypeOptions = [], 13 dateTypeLabel, 14 defaultDateType = '', 15 lastAccessDate, 16 fromLabel, 17 …(truncated, 1408 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx Tool results: → result (ok): 1 import type { Option } from '@doctolib/design-system/Option' 2 3 import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter' 4 import type { FilterConfig } from '../../../types' 5 6 export const createNhrDocumentCategoryFilterConfig = ({ 7 options = [], 8 defaultValue = '', 9 label = 'Document Category', 10 placeholder, 11 }: { 12 options?: Option[] 13 defaultValue?: string 14 label?: string 15 placeholde …(truncated, 626 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.test.tsx Tool results: → result (ok): 1 import { render, screen, waitFor } from '@testing-library/react' 2 // eslint-disable-next-line import/no-extraneous-dependencies, @doctolib-tpf/no-undeclared-dependencies 3 import { userEvent } from '@testing-library/user-event' 4 import { forwardRef } from 'react' 5 import { describe, expect, it, vi } from 'vitest' 6 7 import type { FilterConfig } from '../../types' 8 import { NhrDocumentListFilterPanel } from './NhrDocumentListFilterPanel' 9 10 // Mock design system components 11 vi.mock(' …(truncated, 9750 more chars) Now let me read the remaining changed files. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx Tool results: → result (ok): 1 import { BetaComboBox } from '@doctolib/design-system/BetaComboBox' 2 import type { Option } from '@doctolib/design-system/Option' 3 4 type NhrDocumentCategoryFilterProps = { 5 label: string 6 value: string 7 options: Option[] 8 placeholder?: string 9 onSelectionChange: (option: Option) => void 10 onSearchChange: (search: string) => void 11 } 12 13 export const NhrDocumentCategoryFilter = ({ 14 label, 15 value, 16 options, 17 placeholder, 18 onSelectionChange, 19 onSe …(truncated, 329 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.tsx Tool results: → result (ok): 1 import { Checkbox } from '@doctolib/design-system/Checkbox/v2' 2 import { Heading } from '@doctolib/design-system/Heading' 3 4 import type { NhrDocumentStatusFilterProps } from '../../../types' 5 6 export const NhrDocumentStatusFilter = ({ label, statuses, value, onChange }: NhrDocumentStatusFilterProps) => ( 7 <> 8 {label} 9
10 {statuses.map((status) => ( 11 { 13 const renderComponent = (props: Partial 10 onChange: (filterKey: string, checked: boolean) => void 11 } 12 13 export const NhrDocumentVisibilityFilter = ({ 14 groupLabel, 15 visibilityModes, 16 f …(truncated, 600 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts Tool results: → result (ok): 1 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons' 2 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 3 import type { ReactNode } from 'react' 4 5 export type NhrPatientInfo = { 6 fullName: string 7 gender: 'male' | 'female' | 'other' 8 birthDate: string 9 age: number 10 } 11 12 export type ColumnConfig = { 13 key: string 14 header: string 15 sortable: boolean 16 / Only required when sortable: true. Returns a string for lo …(truncated, 2274 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDateSelectionFilter/NhrDateSelectionFilter.tsx Tool results: → result (ok): 1 import { t } from '#i18n' 2 import type { Option } from '@doctolib/design-system/Option' 3 import { Paragraph } from '@doctolib/design-system/Paragraph' 4 import { RadioGroup } from '@doctolib/design-system/RadioGroup' 5 6 import { NhrDateRangeFilter } from '../NhrDateRangeFilter/NhrDateRangeFilter' 7 import { NhrDateTypeFilter } from '../NhrDateTypeFilter/NhrDateTypeFilter' 8 9 export type DateMode = 'customDateRange' | 'lastAccess' 10 11 type NhrDateSelectionFilterProps = { 12 label: st …(truncated, 2242 more chars) Now let me read the remaining changed files: the storybook files, the test files for the factories, and the preview file. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.stories.tsx Tool results: → result (ok): 1 import type { Meta, StoryObj } from '@storybook/react' 2 import { useState } from 'react' 3 4 import { createNhrDateSelectionFilterConfig } from '../../business/factories/filters/createNhrDateSelectionFilterConfig' 5 import { createNhrDocumentCategoryFilterConfig } from '../../business/factories/filters/createNhrDocumentCategoryFilterConfig' 6 import { createNhrDocumentStatusFilterConfig } from '../../business/factories/filters/createNhrDocumentStatusFilterConfig' 7 import { createNhrDocument …(truncated, 4338 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.test.tsx Tool results: → result (ok): 1 import { t } from '#i18n' 2 import { formatNhrDateTime } from '#utils/nhrDateHelpers' 3 import { setup } from '@doctolib/test-helpers' 4 import { screen } from '@testing-library/react' 5 import { describe, expect, it, vi } from 'vitest' 6 7 import { mockPatientInfo } from '../../../test-helpers' 8 import { createNhrDateSelectionFilterConfig } from './createNhrDateSelectionFilterConfig' 9 10 const ctx = { patient: mockPatientInfo } 11 12 describe('createNhrDateSelectionFilterConfig', () => { …(truncated, 4369 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx Tool results: → result (ok): 1 import { setup } from '@doctolib/test-helpers' 2 import { screen } from '@testing-library/react' 3 import { describe, expect, it, vi } from 'vitest' 4 5 import { mockPatientInfo } from '../../../test-helpers' 6 import { createNhrDocumentCategoryFilterConfig } from './createNhrDocumentCategoryFilterConfig' 7 8 const ctx = { patient: mockPatientInfo } 9 10 describe('createNhrDocumentCategoryFilterConfig', () => { 11 const renderFilter = ( 12 options = [{ value: 'A', label: 'Report A' }] …(truncated, 1631 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/.storybook/preview.tsx Tool results: → result (ok): 1 import '@doctolib/design-system/design-system.scss' 2 import type { Preview } from '@storybook/react' 3 4 import { getMswLoaders, initMsw } from '../../../.storybook/backend-preview' 5 import '../src/dev/index.css' 6 import { getInstance } from '../src/i18n' 7 import './preview.css' 8 9 const i18n = getInstance() 10 11 initMsw() 12 13 // Global flag to ensure i18n is only initialized once across all stories 14 let isI18nInitialized = false 15 16 // One-time i18n initialization loader 17 c …(truncated, 762 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx Tool results: → result (ok): 1 import { setup } from '@doctolib/test-helpers' 2 import { screen } from '@testing-library/react' 3 import { describe, expect, it, vi } from 'vitest' 4 5 import { NhrDocumentCategoryFilter } from './NhrDocumentCategoryFilter' 6 7 const sampleOptions = [ 8 { value: 'A', label: 'Report A' }, 9 { value: 'B', label: 'Report B' }, 10 ] 11 12 describe('NhrDocumentCategoryFilter', () => { 13 const renderComponent = (props: Partial> = {}) …(truncated, 1987 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.stories.tsx Tool results: → result (ok): 1 import type { Meta, StoryObj } from '@storybook/react' 2 3 import { NhrDocumentCategoryFilter } from './NhrDocumentCategoryFilter' 4 5 const meta = { 6 title: 'NHR/Filters/NhrDocumentCategoryFilter', 7 component: NhrDocumentCategoryFilter, 8 tags: ['autodocs'], 9 decorators: [ 10 (Story) => ( 11
12 13
14 ), 15 ], 16 args: { 17 label: 'Cat …(truncated, 731 more chars) Now let me look at the git diff to see what exactly changed in this PR. Tools used: - Bash: git diff main...HEAD -- packages/tpf-national-health-records-ui/ 2>/dev/null | head -600 Tool results: → result (ok): diff --git a/packages/tpf-national-health-records-ui/.storybook/preview.tsx b/packages/tpf-national-health-records-ui/.storybook/preview.tsx index 8d3fe22bb..c05350f04 100644 --- a/packages/tpf-national-health-records-ui/.storybook/preview.tsx +++ b/packages/tpf-national-health-records-ui/.storybook/preview.tsx @@ -2,7 +2,7 @@ import '@doctolib/design-system/design-system.scss' import type { Preview } from '@storybook/react' import { getMswLoaders, initMsw } from '../../../.storybook/backend …(truncated, 26404 more chars) Now let me also check NhrDocumentListModal and the NhrDocumentListTable to see if there were other related changes. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx Tool results: → result (ok): 1 import { Modal } from '@doctolib/design-system/Modal' 2 import { ModalBody } from '@doctolib/design-system/ModalBody' 3 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 4 import { useEffect, useMemo, useState } from 'react' 5 6 import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo, SearchDocumentsFn } from '../../types' 7 import { NhrDocumentList } from '../NhrDocumentList/NhrDocumentList' 8 9 export type NhrDocumentListModalProps = { 10 isOpen: boo …(truncated, 2243 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.tsx Tool results: → result (ok): 1 import { PAGE_SIZE } from '#utils/nhrPaginationHelpers' 2 import { Pagination } from '@doctolib/design-system/Pagination' 3 import { Table } from '@doctolib/design-system/Table' 4 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 5 6 import type { ActionConfig, ColumnConfig } from '../../../types' 7 import { NhrDocumentListTableBody } from './NhrDocumentListTableBody/NhrDocumentListTableBody' 8 import { NhrDocumentListTableHead } from './NhrDocumentListTableHead/NhrDocu …(truncated, 974 more chars) Let me also check the NhrDocumentList.tsx diff more carefully, and verify the NhrDocumentVisibilityFilter changes. Tools used: - Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.tsx Tool results: → result (ok): 1 import { getPagedData } from '#utils/nhrPaginationHelpers' 2 import { SpinnerLoader } from '@doctolib/design-system/SpinnerLoader' 3 import type { NhrDocument } from '@doctolib/tpf-national-health-records' 4 import { useMemo, useState } from 'react' 5 6 import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo } from '../../types' 7 import { NhrDocumentListFilterPanel } from '../NhrDocumentListFilterPanel/NhrDocumentListFilterPanel' 8 import { NhrDocumentListHeader } from '../Nhr …(truncated, 3113 more chars) Now let me also look at the previous version of createNhrDocumentCategoryFilterConfig.tsx from the diff to understand the breaking change (positional params to object param). Tools used: - Bash: git diff main...HEAD -- packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx 2>/dev/null Tool results: → result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx index dfa465d6a..0c9ce42e8 100644 --- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx +++ b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFil …(truncated, 3584 more chars) Now I have a comprehensive understanding of all the changes. Let me also check the NhrDocumentVisibilityFilter diff. Tools used: - Bash: git diff main...HEAD -- packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx 2>/dev/null Tool results: → result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx index fd031798d..310479f13 100644 --- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx +++ b/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocument …(truncated, 358 more chars) Tools used: - Bash: git diff main...HEAD -- packages/tpf-national-health-records-ui/src/types.ts 2>/dev/null Tool results: → result (ok): diff --git a/packages/tpf-national-health-records-ui/src/types.ts b/packages/tpf-national-health-records-ui/src/types.ts index 542ac545e..a5985c72d 100644 --- a/packages/tpf-national-health-records-ui/src/types.ts +++ b/packages/tpf-national-health-records-ui/src/types.ts @@ -27,6 +27,8 @@ export type StatusOption = { filterKey: string label: string isDisabled?: boolean + / When defined, overrides the computed checked state from the filter value array / + isSelected?: boolean } …(truncated, 33 more chars) I now have a thorough understanding of all the changes in this PR. Let me compile my review findings. --- Code Review: PR #2935 -- "feat(fhir-2190)!: improve NHR document list filter panel UI" Files Reviewed - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.stories.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/utils/nhrDateHelpers.test.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDateSelectionFilterConfig.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.stories.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.test.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDateSelectionFilter/NhrDateSelectionFilter.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.tsx - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts - /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/.storybook/preview.tsx Project Rules Checked - .claude/rules/general.md (Conventional Commits, Git hooks) - .claude/rules/frontend/react/react.md (Code style, imports, Oxygen design system) - .claude/rules/frontend/react/test/testing.md (Vitest, RTL, MSW patterns) - .claude/rules/frontend/react/test/testing-patterns.md (Query priority, prohibited patterns) - .claude/rules/frontend/react/oxygen-design-system.md (v2 components, import paths) - .claude/rules/frontend/storybook.md (Story structure, preview setup) - .claude/rules/frontend/structure/component-patterns.md (Named exports, directory structure) - .claude/rules/testing/devex.md (Prohibited patterns, naming) --- Important Issues (Confidence 80-89) 1. Accessibility: Backdrop uses role='button' for a non-button overlay element -- Confidence: 82 File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx, line 122-129 The backdrop overlay
uses role='button' with tabIndex={0}. This is semantically incorrect for a modal backdrop/overlay. The role='button' makes the entire viewport-sized overlay appear as a clickable button to screen readers, which is misleading. A more appropriate pattern for a modal overlay would be to use role='dialog' on the panel itself or an aria-modal attribute, and handle the backdrop click without advertising the backdrop as a button. This is a pre-existing pattern (not introduced by this PR), but the PR did change the width from 400px to 528px without addressing this. Noting it for awareness but it may be out of scope for this PR. 2. Test uses CSS selector / data-testid for structural assertions instead of semantic queries -- Confidence: 80 File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.test.tsx, lines 88-93 and 95-113 The tests at lines 88-93 use container.querySelector('[data-testid="filter-panel-footer-actions"]') to query the DOM, and lines 95-113 use container.querySelectorAll('[data-filter-key]'). The project rules in .claude/rules/frontend/react/test/testing-patterns.md state: > Prohibited for selection: CSS selectors (.class, #id) -- prefer semantic queries And .claude/rules/testing/quick-reference.md: > Prohibited Patterns: CSS selectors (.class, #id) -- prefer semantic queries However, these tests are verifying CSS class names and structural layout (gap-16, p-16) which is explicitly listed as an acceptable exception in testing-patterns.md: > ACCEPTABLE: Testing CSS class application -- Testing that error CSS class is applied Given that these tests are asserting on Tailwind classes (structural styling), this is borderline acceptable. The data-testid selector for filter-panel-footer-actions is specifically intended for testing, which is a standard practice. The data-filter-key selector is also a custom data attribute added for testing purposes. These fall within the allowed exceptions, but adding a brief comment justifying the CSS selector usage would align better with the project's documented testing patterns. --- Summary This is a well-structured PR with clean code changes. The key observations: What looks good: - The Oxygen Divider component is correctly imported from @doctolib/design-system/Divider, following the design system rules. - Button/v2 and IconButton/v2 are used as required by the Oxygen design system guidelines. - The refactoring of createNhrDocumentCategoryFilterConfig from positional params to a single object param is a clean API improvement. The breaking change is properly signaled with ! in the PR title per Conventional Commits. - formatNhrDateTime is a well-isolated pure function with comprehensive test coverage (null, undefined, empty string, invalid dates, valid dates, edge cases like midnight and 23:59). - The test file for nhrDateHelpers.test.ts (133 lines) is well within the 400-line guideline and follows the nested describe pattern recommended by the project. - The NhrDocumentStatusFilter use of status.isSelected ?? value.includes(status.filterKey) is correct: nullish coalescing ensures that when isSelected is explicitly false, it overrides the computed value (unlike || which would fall through on false). The corresponding tests at lines 66-82 in NhrDocumentStatusFilter.test.tsx verify both isSelected: true and isSelected: false scenarios. - Formatting lastAccessDate in the factory (createNhrDateSelectionFilterConfig) rather than the presentational component (NhrDateSelectionFilter) is a reasonable architectural choice -- the factory already performs data transforma …(truncated, 825 more chars)