Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-sonnet-4-6`
Stats: 1 user messages, 47 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review the latest changes in branch fhir-2219_implement-multi-select-category-filter of the treatment-plan repo at /Users/USER/workspace/doctolib/treatment-plan.
Get the diff with: git diff main...HEAD
This is a second-round review. Previous findings that were fixed:
- triggerLabel computed in factory → moved to component
- Missing index.ts → added
- placeholder unused → now used
- Raw × → replaced with XmarkLargeRegularIcon
- Path alias (#components) → NOT available in this package's imports config, skipped
Focus only on remaining or new issues. The custom dropdown (no AlphaMultipleComboBox available in exports) is an approved implementation approach — do NOT re-flag this as an issue.
Turn 1 — Assistant
I'll start by getting the diff and exploring the package structure to perform a thorough review.
Tools used:
Bash:git diff main...HEAD --name-only
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.stories.tsx packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.test.tsx pa …(truncated, 230 more chars)
Tools used:
Bash:git log main..HEAD --oneline
Tool results:
→ result (ok): 813c13834 refactor(fhir-2219): address code review findings 755f86d2d feat(fhir-2219): implement multi-select for Document Category filter
Tools used:
Bash:git diff main...HEAD
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx b/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx index cabf73685..18a0464f3 100644 --- a/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.test.tsx +++ b/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocum …(truncated, 19829 more chars)
Now let me examine the component implementation and related files more closely.
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 { ChevronDownRegularIcon } from '@doctolib/design-system-icons/ChevronDownRegularIcon'
2 import { XmarkLargeRegularIcon } from '@doctolib/design-system-icons/XmarkLargeRegularIcon'
3 import { Checkbox } from '@doctolib/design-system/Checkbox/v2'
4 import type { Option } from '@doctolib/design-system/Option'
5 import { useEffect, useRef, useState } from 'react'
6
7 type NhrDocumentCategoryFilterProps = {
8 label: string
9 values: string[]
10 options: Option[]
11 placeholder?: st
…(truncated, 2503 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 label = 'Document Category',
9 placeholder,
10 }: {
11 options?: Option[]
12 label?: string
13 placeholder?: string
14 } = {}): FilterConfig => ({
15 key:
…(truncated, 679 more chars)
Now let me look at the broader context — the component directory naming and the factory file's import path.
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/
Tool results:
→ result (ok): NhrDocumentCategoryCell NhrDocumentCategoryFilter NhrDocumentList.stories.tsx NhrDocumentList.test.tsx NhrDocumentList.tsx NhrDocumentNameCell NhrDocumentStatusFilter NhrProcedureDateCell NhrShareDateCell NhrSharedByCell NhrStatusCell NhrVisibilityCell
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/
Tool results:
→ result (ok): createNhrDateSelectionFilterConfig.test.tsx createNhrDateSelectionFilterConfig.tsx createNhrDocumentCategoryFilterConfig.test.tsx createNhrDocumentCategoryFilterConfig.tsx createNhrDocumentStatusFilterConfig.test.tsx createNhrDocumentStatusFilterConfig.tsx createNhrDocumentVisibilityFilterConfig.test.tsx createNhrDocumentVisibilityFilterConfig.tsx
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/
Tool results:
→ result (ok): NhrDocumentStatusFilter.stories.tsx NhrDocumentStatusFilter.test.tsx NhrDocumentStatusFilter.tsx
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentStatusFilterConfig.tsx
Tool results:
→ result (ok): 1 import { NhrDocumentStatusFilter } from '../../../components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter'
2 import type { FilterConfig, StatusOption } from '../../../types'
3
4 export const createNhrDocumentStatusFilterConfig = (statuses: StatusOption[], label: string): FilterConfig => ({
5 key: 'documentStatus',
6 render: (filters, onChange) => (
7 <NhrDocumentStatusFilter
8 label={label}
9 statuses={statuses}
10 value={statuses.filter((s) => fil
…(truncated, 158 more chars)
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/
Tool results:
→ result (ok): NhrDocumentStatusFilter.stories.tsx NhrDocumentStatusFilter.test.tsx NhrDocumentStatusFilter.tsx
Tools used:
Bash:grep -r "NhrDocumentCategoryFilter" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src --include="*.ts" --include="*.tsx" -l
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/index.ts /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-rec …(truncated, 772 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx(1–6)
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
Tools used:
Bash:grep "NhrDocumentCategoryFilter" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/index.ts
Tool results:
→ result (ok): export { createNhrDocumentCategoryFilterConfig } from './business/factories/filters/createNhrDocumentCategoryFilterConfig'
Tools used:
Bash:grep "NhrDocumentStatusFilter\|NhrDocumentCategoryFilter" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentSta …(truncated, 19 more chars)
Tool results:
→ result (ok): import { NhrDocumentStatusFilter } from '../../../components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter' export const createNhrDocumentStatusFilterConfig = (statuses: StatusOption[], label: string): FilterConfig => ({ <NhrDocumentStatusFilter
The status filter factory imports directly by filename (NhrDocumentStatusFilter) since there is no index.ts in that folder. The category filter factory does the same — consistent. Now let me check the displayLabel logic more closely, and inspect the aria-expanded attribute type.
Tools used:
Bash:grep -n "aria-expanded" packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx
Tool results:
→ result (ok): 62: aria-expanded={isOpen}
Tools used:
Bash:npm run type-check 2>&1 | grep -A2 "NhrDocumentCategoryFilter" | head -40
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:npm run type-check 2>&1 | tail -20
Tool results:
→ result (ok): zsh: command not found: npm
Tools used:
Bash:npm run type-check 2>&1 | tail -20
Tool results:
→ result (ok): zsh: command not found: npm
Tools used:
Bash:/Users/USER/.nvm/versions/node/$(cat .node-version)/bin/npm run type-check 2>&1 | tail -30
Tool results:
→ result (ok): (eval):1: no such file or directory: /Users/USER/.nvm/versions/node/24.14.1/bin/npm
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/.node-version
Tool results:
→ result (ok): 24.14.1
Tools used:
Bash:which node || ls ~/.nvm/versions/node/ 2>/dev/null | head -5 || ls ~/.mise/installs/node/ 2>/dev/null | head -5
Tool results:
→ result (ok): node not found
Tools used:
Bash:ls ~/.mise/installs/node/ 2>/dev/null || ls /usr/local/bin/node 2>/dev/null || find /Users/clertonra -name "node" -type f 2>/dev/null | head -5
Tool results:
→ result (ok): Command running in background with ID: bd5942dbg. Output is being written to: /private/tmp/claude-501/-Users-clertonra-workspace-doctolib/aaee6e45-36b2-40ec-bc58-e9c07ba098ea/tasks/bd5942dbg.output
Tools used:
Read:/private/tmp/claude-501/-Users-clertonra-workspace-doctolib/aaee6e45-36b2-40ec-bc58-e9c07ba098ea/tasks/bd5942dbg.output
Tool results:
→ result (ok): <system-reminder>Warning: the file exists but is shorter than the provided offset (1). The file has 1 lines.</system-reminder>
Tools used:
Bash:find /Users/USER/.mise -name "npm" -type f 2>/dev/null | head -3
Tool results:
→ result (ok): (Bash completed with no output)
Type-check tools are not available in this shell environment. Let me focus the review on what can be statically analyzed from the diff and source code.
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 { value: 'C', label: 'Report C' },
11 ]
12
13 describe('NhrDocumentCategoryFilter', () => {
14 const renderComponent = (props: Partial<React.ComponentProps<t
…(truncated, 5169 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/DocumentList/Filters/DocumentCategoryFilter',
7 component: NhrDocumentCategoryFilter,
8 tags: ['autodocs'],
9 decorators: [
10 (Story) => (
11 <div data-theme='doctolib2023' style={{ height: '40vh', padding: '24px', maxWidth: '400px' }}>
12 <Story />
13 </div>
14 ),
15 ],
16 args: {
17 l
…(truncated, 919 more chars)
Now let me check a few specific details — the aria-expanded boolean vs string issue, the displayLabel edge case when selected values don't match any option, and the factory import path vs index pattern.
Tools used:
Bash:grep -n "aria-expanded" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.ts …(truncated, 1 more chars)
Tool results:
→ result (ok): 62: aria-expanded={isOpen}
Tools used:
Bash:grep -rn "from.*NhrDocumentCategoryFilter/NhrDocumentCategoryFilter\|from.*NhrDocumentCategoryFilter'" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ - …(truncated, 33 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx:3:import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/index.ts:1:export * from './NhrDocumentCategoryFilter' /Users/US …(truncated, 497 more chars)
Tools used:
Bash:grep -rn "from.*NhrDocumentStatusFilter/NhrDocumentStatusFilter\|from.*NhrDocumentStatusFilter'" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ --inclu …(truncated, 27 more chars)
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentStatusFilterConfig.tsx:1:import { NhrDocumentStatusFilter } from '../../../components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/NhrDocumentStatusFilter.test.tsx:5:import { NhrDocumentStatusFilter } from …(truncated, 275 more chars)
Tools used:
Bash:grep -n "defaultValue\|placeholder" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
Tool results:
→ result (ok): 9: placeholder, 13: placeholder?: string 24: placeholder={placeholder}
Tools used:
Bash:grep -rn "aria-haspopup\|role.*listbox\|role.*combobox" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFil …(truncated, 16 more chars)
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -rn "aria-haspopup\|role.*listbox" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter/ 2>/dev/null
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -rn "import.*React\b" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter …(truncated, 4 more chars)
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -rn "React\." /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx:46: const handleKeyDown = (e: React.KeyboardEvent) => { /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx:50: const handleClear = (e: React.MouseEvent) => {
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/tsconfig.json 2>/dev/null || cat /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national …(truncated, 60 more chars)
Tool results:
→ result (ok): { "extends": "../../tsconfig-base", "include": [ "../../vitest-shared.config.ts", "../../vitest-setup", "/*", "src/locales//.json", "package.json", "../../.storybook/.ts" ], "compilerOptions": { "types": ["vitest/globals", "@testing-library/jest-dom"] } }
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/tsconfig-base.json 2>/dev/null | head -30
Tool results:
→ result (ok): { "extends": ["@doctolib/upfront-dev/tsconfig.json"], "compilerOptions": { // Configuration for TS project references "rootDir": ".", "outDir": "build", "composite": true, "noEmit": false, "declaration": true, // need to set this back to false to allow noEmit: false "allowImportingTsExtensions": false, // was needed for webpack build "jsx": "react-jsx",
// ignore errors due to design system
"exactOptionalPropertyTypes": false,
"types": ["
…(truncated, 507 more chars)
The react-jsx transform means React is auto-imported for JSX, but React.KeyboardEvent and React.MouseEvent as type references still require React to be in scope. With react-jsx, the global React namespace is not automatically available. Let me verify how other components handle this.
Tools used:
Bash:grep -rn "React\.KeyboardEvent\|React\.MouseEvent\|React\.ChangeEvent" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ --include="*.tsx" | head -10
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDateRangeFilter/NhrDateRangeFilter.test.tsx:15: }: { label: string; onChange: React.ChangeEventHandler<HTMLInputElement>; value: string; name: string }, /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx:46: const handleKeyDown = (e: React.KeyboardEvent) => { /Users/USE …(truncated, 659 more chars)
Tools used:
Bash:grep -rn "^import.*React" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ --include="*.tsx" | head -10
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -n "^import" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListFilterPanel/NhrDocumentListFilterPanel.tsx | head -10
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' 8:import type { FilterConfig } from '../../types'
Tools used:
Bash:grep -n "upfront-dev" /Users/USER/workspace/doctolib/treatment-plan/node_modules/@doctolib/upfront-dev/tsconfig.json 2>/dev/null || cat /Users/USER/workspace/doctolib/treatment-plan/node_mod …(truncated, 63 more chars)
Tool results:
→ result (ok): { "extends": ["./bases/tsconfig.json"], "include": ["src", "tests", "*.ts"] }
Tools used:
Bash:cat /Users/USER/workspace/doctolib/treatment-plan/node_modules/@doctolib/upfront-dev/bases/tsconfig.json 2>/dev/null | head -30
Tool results:
→ result (ok): // A core TS configuration for UpFront packages
//
// @see https://www.typescriptlang.org/tsconfig
{
// Let's start off strong. Includes strict: true.
"extends": ["@tsconfig/recommended/tsconfig.json"],
// Now let's tweak and augment that.
"compilerOptions": {
// Enable completion and type-safety for CSS modules (CSS/SCSS)
// Works hand in hand with asset typings (which we provide in assets.d.ts)
"plugins": [
{
"name": "typescript-plugin-css-modules"
}
…(truncated, 796 more chars)
Tools used:
Bash:grep -n "jsx\|react\|global" /Users/USER/workspace/doctolib/treatment-plan/node_modules/@doctolib/upfront-dev/bases/tsconfig.json 2>/dev/null
Tool results:
→ result (ok): 33: // Don't treat any module as global just because it doesnt import/export. 44: "jsx": "react-jsx",
The react-jsx transform injects jsx runtime without adding React to scope, but NhrDocumentListFilterPanel uses React.MouseEvent and React.KeyboardEvent without importing React either — and that file predates this PR. This is an existing pattern in the package, not introduced here. The @types/react package with react-jsx does provide the React namespace as a global in @types/react >= 17, but it's worth flagging as it depends on that declaration ambient setup. Since the sibling file has the same pattern without issues, this is not a new problem introduced by this PR.
Let me now check the displayLabel logic for the case where values are present but none match any option.
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx(38–45)
Tool results:
→ result (ok): 38 const firstLabel = options.find((o) => values.includes(o.value))?.label ?? ''
39 const displayLabel =
40 values.length === 0
41 ? (placeholder ?? '')
42 : values.length === 1
43 ? firstLabel
44 : ${firstLabel}, +${values.length - 1}
Now let me check the factory's import — it bypasses the index.ts and imports directly from the .tsx file. Let me confirm whether the sibling status filter follows the same pattern and whether there is a convention established in this package for intra-package imports.
Tools used:
Bash:grep -rn "from.*NhrDocument.*Filter/NhrDocument.*Filter\b" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/ --include="*.tsx" --include="*.ts"
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/index.ts:61:} from './components/NhrDocumentVisibilityFilter/NhrDocumentVisibilityFilter' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx:3:import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter' /Users/USER/workspace/docto …(truncated, 542 more chars)
The pattern of bypassing index.ts and importing directly from the .tsx file is consistent with all other factory files in the package. The NhrDocumentCategoryFilter factory follows this established convention exactly.
Now let me check the ChevronDownRegularIcon — it's rendered outside the button but is a UI affordance for the dropdown trigger. This is a minor accessibility concern.
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx(55–91)
Tool results:
→ result (ok): 55 return ( 56 <div ref={containerRef} className='relative w-full'> 57 <span className='text-sm font-medium'>{label}</span> 58 <div className='flex items-center border rounded'> 59 <button 60 type='button' 61 aria-label={label} 62 aria-expanded={isOpen} 63 className='flex-1 text-left px-2 py-1' 64 onClick={() => setIsOpen((open) => !open)} 65 onKeyDown={handleKeyDown} 66 > 67 {displayLabel} 68 …(truncated, 745 more chars)
Tools used:
Bash:grep -rn "aria-controls\|role.*listbox\|aria-haspopup" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentStatusFilter …(truncated, 19 more chars)
Tool results:
→ result (ok): (Bash completed with no output)
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 <Heading level='h5'>{label}</Heading>
9 <div className='flex flex-col'>
10 {statuses.map((status) => (
11 <Checkbox
12 key={status.filterKey}
13
…(truncated, 327 more chars)
The sibling NhrDocumentStatusFilter is always-open (no dropdown toggle), so the ARIA comparison is not directly relevant. Now I have enough information for the full review.
Here is the second-round review.
Architecture Review: fhir-2219_implement-multi-select-category-filter
Critical Issues (must fix)
None. The core architecture violations from round one (missing index.ts, unused placeholder, raw × character, triggerLabel in factory) are all resolved. The custom dropdown implementation is approved.
Warnings (should fix)
1. ChevronDownRegularIcon sits outside the trigger <button>, breaking the click target
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx, line 74.
The chevron icon is rendered as a sibling of the <button> and the Clear <button>, inside a plain <div>. Clicking the chevron does not toggle the dropdown because the onClick is on the <button, not the wrapping <div>. The icon is also not decorated with aria-hidden, so screen readers will announce it as content. Move the chevron inside the trigger button, or move onClick to the wrapper <div> with an appropriate role and keyboard handler.
Minimal fix — move the icon inside the button:
<button
type='button'
aria-label={label}
aria-expanded={isOpen}
className='flex-1 text-left px-2 py-1 flex items-center justify-between'
onClick={() => setIsOpen((open) => !open)}
onKeyDown={handleKeyDown}
>
{displayLabel}
<ChevronDownRegularIcon aria-hidden />
</button>
This is flagged as a warning rather than critical because the test suite covers open/close via button clicks and the chevron is purely visual, but a user who clicks the icon will get no response, which is a real interaction bug.
2. displayLabel shows ", +N" when selected values don't match any option
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx, lines 38–44.
When values is non-empty but none of the values match any option, firstLabel resolves to ''. The display logic then renders ', +N' for multiple unmatched values (e.g. values = ['UNKNOWN_A', 'UNKNOWN_B'] produces ', +1'). The factory-level test at line 54 of the test file only covers the single-unmatched case, which correctly shows nothing. The multi-unmatched case is untested and produces a confusing label.
The fix is to guard on firstLabel:
const displayLabel =
values.length === 0 || firstLabel === ''
? (placeholder ?? '')
: values.length === 1
? firstLabel
: `${firstLabel}, +${values.length - 1}`
Add a matching test: 'should show empty trigger when multiple selected typeCodes do not match any option'.
3. Factory imports directly from .tsx rather than through index.ts
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx, line 3.
import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter'
An index.ts was added in this PR precisely to act as the re-exporter. The factory should import through the index.ts entrypoint, not directly from the implementation file. This is the purpose of the re-exporter pattern.
import { NhrDocumentCategoryFilter } from '../../../components/NhrDocumentList/NhrDocumentCategoryFilter'
Note: the sibling factories (createNhrDocumentStatusFilterConfig, createNhrDocumentVisibilityFilterConfig) do the same direct import, but those components do not have an index.ts. The category filter does. Since the index.ts was a previous-round fix, it should be used.
Suggestions (consider improving)
4. onKeyDown is attached to the trigger button but not to the dropdown panel
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx, line 65.
The Escape handler lives on the trigger button only. Once focus moves to a checkbox inside the dropdown panel, pressing Escape will not close the dropdown. The test at line 130 only verifies Escape closes the dropdown when focus is still on the trigger (the user just clicked the button). Consider moving the onKeyDown handler to the outer <div ref={containerRef}> so all focus within the component captures Escape.
5. aria-expanded missing aria-haspopup
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx, line 62.
aria-expanded on a <button> is meaningful to screen readers only when paired with aria-haspopup. Without it, assistive technologies do not know what the expanded state refers to. Add aria-haspopup='listbox' to the trigger button.
6. The dropdown panel has no ARIA role
File: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx, line 77.
The floating <div> containing the checkboxes has no semantic role. Adding role='listbox' or role='group' with an aria-label would make the structure comprehensible to assistive technology.