Project: workspace-doctolib · Branch: HEAD · Mode: ` · **Model:**claude-opus-4-7`
Stats: 1 user messages, 30 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review the recently modified code on branch fhir-2219_implement-multi-select-category-filter in the treatment-plan repo at /Users/USER/workspace/doctolib/treatment-plan for simplification opportunities.
Run git diff origin/main...HEAD to see all changes.
Context
The changed files are all in packages/tpf-national-health-records-ui/src/:
- components/MultiSelectDropdown/ — new custom multi-select dropdown component split into sub-components
- components/NhrDocumentList/NhrDocumentCategoryFilter/ — thin wrapper
- business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx — factory
Focus on: redundant code, overly complex logic that could be simplified, duplicated patterns between sub-components (especially between MultiSelectDropdownPopup.test.tsx and MultiSelectDropdownOptions.test.tsx which are nearly identical), and any code that could be cleaner while preserving all functionality. Do NOT simplify test files to the point of losing coverage. Report only actionable findings.
Turn 1 — Assistant
I'll analyze the recently modified code on the branch for simplification opportunities.
Tools used:
Bash:git diff origin/main...HEAD --stat
Tool results:
→ result (ok): .../tpf-national-health-records-ui/package.json | 1 +
.../createNhrDocumentCategoryFilterConfig.test.tsx | 120 +++++--
.../createNhrDocumentCategoryFilterConfig.tsx | 19 +-
.../MultiSelectDropdown.stories.tsx | 83 +++++
.../MultiSelectDropdown.test.tsx | 393 +++++++++++++++++++++
.../MultiSelectDropdown/MultiSelectDropdown.tsx | 88 +++++
.../MultiSelectDropdownLabel.test.tsx | 35 ++
.../MultiSelectDropdownLabel.tsx
…(truncated, 1004 more chars)
Tools used:
Bash:git branch --show-current
Tool results:
→ result (ok): fhir-2219_implement-multi-select-category-filter
Tools used:
Bash:ls /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/
Tool results:
→ result (ok): helpers.test.ts helpers.ts hooks.test.tsx hooks.ts index.ts MultiSelectDropdown.stories.tsx MultiSelectDropdown.test.tsx MultiSelectDropdown.tsx MultiSelectDropdownLabel.test.tsx MultiSelectDropdownLabel.tsx MultiSelectDropdownOptions.test.tsx MultiSelectDropdownOptions.tsx MultiSelectDropdownPopup.test.tsx MultiSelectDropdownPopup.tsx MultiSelectDropdownTrigger.test.tsx MultiSelectDropdownTrigger.tsx
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdown.tsx
Tool results:
→ result (ok): 1 import type { Option } from '@doctolib/design-system/Option'
2 import { useId, useMemo, useRef, useState } from 'react'
3
4 import { MultiSelectDropdownLabel } from './MultiSelectDropdownLabel'
5 import { MultiSelectDropdownPopup } from './MultiSelectDropdownPopup'
6 import { MultiSelectDropdownTrigger } from './MultiSelectDropdownTrigger'
7 import { normalize } from './helpers'
8 import { useDropdownClose } from './hooks'
9
10 type MultiSelectDropdownProps = {
11 label: string
12 values
…(truncated, 2252 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownLabel.tsx
Tool results:
→ result (ok): 1 type MultiSelectDropdownLabelProps = {
2 htmlFor: string
3 label: string
4 }
5
6 export const MultiSelectDropdownLabel = ({ htmlFor, label }: MultiSelectDropdownLabelProps) => (
7 <label htmlFor={htmlFor} className='oxygen-input-field__label mb-1 block'>
8 {label}
9 </label>
10 )
11
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownPopup.tsx
Tool results:
→ result (ok): 1 import type { Option } from '@doctolib/design-system/Option'
2
3 import { MultiSelectDropdownOptions } from './MultiSelectDropdownOptions'
4
5 type MultiSelectDropdownPopupProps = {
6 id: string
7 label: string
8 options: Option[]
9 values: string[]
10 onToggle: (value: string) => void
11 }
12
13 export const MultiSelectDropdownPopup = ({ id, label, options, values, onToggle }: MultiSelectDropdownPopupProps) => (
14 <fieldset
15 id={id}
16 role='dialog'
17 aria-label
…(truncated, 412 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownOptions.tsx
Tool results:
→ result (ok): 1 import { Checkbox } from '@doctolib/design-system/Checkbox/v2'
2 import type { Option } from '@doctolib/design-system/Option'
3
4 type MultiSelectDropdownOptionsProps = {
5 options: Option[]
6 values: string[]
7 onToggle: (value: string) => void
8 }
9
10 export const MultiSelectDropdownOptions = ({ options, values, onToggle }: MultiSelectDropdownOptionsProps) => (
11 <>
12 {options.map((option) => (
13 <Checkbox
14 key={option.value}
15 name={option.value}
1
…(truncated, 269 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownTrigger.tsx
Tool results:
→ result (ok): 1 import { ChevronDownRegularIcon } from '@doctolib/design-system-icons/ChevronDownRegularIcon'
2 import { IconButton } from '@doctolib/design-system/IconButton/v2'
3
4 type MultiSelectDropdownTriggerProps = {
5 triggerId: string
6 popupId: string
7 isOpen: boolean
8 displayLabel: string
9 searchQuery: string
10 placeholder?: string
11 hasValues: boolean
12 onInputChange: (value: string) => void
13 onInputClick: () => void
14 onClear: () => void
15 }
16
17 export const Mult
…(truncated, 1459 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/helpers.ts
Tool results:
→ result (ok): 1 export const normalize = (s: string) => 2 s 3 .normalize('NFD') 4 .replace(/\p{Diacritic}/gu, '') 5 .toLowerCase() 6
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.ts
Tool results:
→ result (ok): 1 import { type RefObject, useEffect, useRef } from 'react'
2
3 export const useDropdownClose = (
4 containerRef: RefObject<HTMLDivElement | null>,
5 onClose: () => void,
6 isOpen: boolean
7 ) => {
8 const onCloseRef = useRef(onClose)
9 // Sync ref so the stable event listeners always call the latest onClose.
10 useEffect(() => {
11 onCloseRef.current = onClose
12 }, [onClose])
13
14 useEffect(() => {
15 if (!isOpen) return undefined
16
17 const handlePointerDown =
…(truncated, 625 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/index.ts
Tool results:
→ result (ok): 1 export * from './MultiSelectDropdown' 2
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdown.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import type { ComponentProps } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { MultiSelectDropdown } from './MultiSelectDropdown'
7
8 const sampleOptions = [
9 { value: 'A', label: 'Report A' },
10 { value: 'B', label: 'Report B' },
11 { value: 'C', label: 'Report C' },
12 ]
13
14 describe('MultiSelectDropdown', () => {
15 afterEach(() => {
16
…(truncated, 16079 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownPopup.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import type { ComponentProps } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { MultiSelectDropdownPopup } from './MultiSelectDropdownPopup'
7
8 describe('MultiSelectDropdownPopup', () => {
9 afterEach(() => {
10 vi.clearAllMocks()
11 vi.restoreAllMocks()
12 })
13
14 const sampleOptions = [
15 { value: 'A', label: 'Report A' },
16 {
…(truncated, 4172 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownOptions.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import type { ComponentProps } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { MultiSelectDropdownOptions } from './MultiSelectDropdownOptions'
7
8 describe('MultiSelectDropdownOptions', () => {
9 afterEach(() => {
10 vi.clearAllMocks()
11 vi.restoreAllMocks()
12 })
13
14 const sampleOptions = [
15 { value: 'A', label: 'Report A' },
16
…(truncated, 3317 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownLabel.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import type { ComponentProps } from 'react'
4 import { describe, expect, it } from 'vitest'
5
6 import { MultiSelectDropdownLabel } from './MultiSelectDropdownLabel'
7
8 describe('MultiSelectDropdownLabel', () => {
9 const renderComponent = (props: Partial<ComponentProps<typeof MultiSelectDropdownLabel>> = {}) =>
10 setup(<MultiSelectDropdownLabel htmlFor='my-input' label='Document type'
…(truncated, 837 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownTrigger.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import type { ComponentProps } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { MultiSelectDropdownTrigger } from './MultiSelectDropdownTrigger'
7
8 describe('MultiSelectDropdownTrigger', () => {
9 afterEach(() => {
10 vi.clearAllMocks()
11 vi.restoreAllMocks()
12 })
13
14 const renderComponent = (props: Partial<ComponentProps<typeof MultiS
…(truncated, 4267 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/helpers.test.ts
Tool results:
→ result (ok): 1 import { describe, expect, it } from 'vitest'
2
3 import { normalize } from './helpers'
4
5 describe('normalize', () => {
6 it('should return an empty string for empty input', () => {
7 expect(normalize('')).toBe('')
8 })
9
10 it('should lowercase ASCII text', () => {
11 expect(normalize('HELLO')).toBe('hello')
12 })
13
14 it('should strip French diacritics', () => {
15 expect(normalize('Médecin')).toBe('medecin')
16 })
17
18 it('should strip German diacritics',
…(truncated, 351 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import { useRef } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { useDropdownClose } from './hooks'
7
8 const DropdownCloseFixture = ({ isOpen, onClose }: { isOpen: boolean; onClose: () => void }) => {
9 const containerRef = useRef<HTMLDivElement>(null)
10 useDropdownClose(containerRef, onClose, isOpen)
11 return (
12 <div ref={containerRef} da
…(truncated, 3202 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdown.stories.tsx
Tool results:
→ result (ok): 1 import type { Meta, StoryObj } from '@storybook/react'
2 import { useState } from 'react'
3
4 import { MultiSelectDropdown } from './MultiSelectDropdown'
5
6 const meta = {
7 title: 'Components/MultiSelectDropdown',
8 component: MultiSelectDropdown,
9 tags: ['autodocs'],
10 decorators: [
11 (Story) => (
12 <div data-theme='doctolib2023' style={{ height: '40vh', padding: '24px', maxWidth: '400px' }}>
13 <Story />
14 </div>
15 ),
16 ],
17 args: {
18
…(truncated, 1790 more chars)
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 { MultiSelectDropdown } from '#components/MultiSelectDropdown'
2 import type { ComponentProps } from 'react'
3
4 export type NhrDocumentCategoryFilterProps = ComponentProps<typeof MultiSelectDropdown>
5
6 export const NhrDocumentCategoryFilter = (props: NhrDocumentCategoryFilterProps) => <MultiSelectDropdown {...props} />
7
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 type { ComponentProps } from 'react'
4 import { afterEach, describe, expect, it, vi } from 'vitest'
5
6 import { NhrDocumentCategoryFilter } from './NhrDocumentCategoryFilter'
7
8 const sampleOptions = [
9 { value: 'A', label: 'Report A' },
10 { value: 'B', label: 'Report B' },
11 ]
12
13 describe('NhrDocumentCategoryFilter', () => {
14 afterEach(() => {
15 vi.clearAllMocks()
…(truncated, 1961 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/index.ts
Tool results:
→ result (ok): 1 export * from './NhrDocumentCategoryFilter' 2
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)
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 { NhrDocumentCategoryFilter } from '#components/NhrDocumentList/NhrDocumentCategoryFilter'
2 import type { Option } from '@doctolib/design-system/Option'
3
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: 'typeCodes',
16 render: (filter
…(truncated, 651 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 { afterEach, 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 const sampleOptions = [
11 { value: 'A', label: 'Report A' },
12 { value: 'B', label: 'Report B' },
13 { value: 'C', labe
…(truncated, 4905 more chars)
Now let me check what the FilterConfig type expects to understand the _ctx parameter.
Tools used:
Bash:find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src -name "types*.ts" -o -name "types.tsx" | head -5
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts
Tools used:
Bash:grep -n "FilterConfig\|render" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts | head -40
Tool results:
→ result (ok): 18: render: (doc: NhrDocument) => ReactNode 21:/ Passed to every FilterConfig.render() — country variant casts to its own patient type */ 47:/ One filter control definition — render patches only its own key(s) via onChange / 48:export type FilterConfig = { 50: render: ( 83: / Optional icon rendered before the label / 87: / When provided, the option is only rendered for rows where this returns true. If not provided, it'll always be available. */ 93:/ One action button definition — …(truncated, 49 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts(47–62)
Tool results:
→ result (ok): 47 / One filter control definition — render patches only its own key(s) via onChange */
48 export type FilterConfig = {
49 key: string
50 render: (
51 / generic — DMP variant casts to DmpServerFilters */
52 filters: Record<string, unknown>,
53 onChange: (patch: Record<string, unknown>) => void,
54 ctx: FilterRenderContext
55 ) => ReactNode
56 }
57
58 export type SearchDocumentsResult = {
59 data: NhrDocument[]
60 total?: number
61 status: NhrResultStatus
Now let me check git diff to confirm what NhrDocumentCategoryFilter looked like before this change, since the task mentions it's a "thin wrapper" now.
Tools used:
Bash:git diff origin/main...HEAD -- packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx | head -80
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx index c7abe6e86..7245ab876 100644 --- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx +++ b/packages/tpf-national-health-records-ui/src/components/Nhr …(truncated, 1184 more chars)
Now let me also check how useDropdownClose is used in the test and if the ref pattern is necessary.
Tools used:
Bash:grep -rn "useDropdownClose" /Users/USER/workspace/doctolib/treatment-plan/packages/ 2>/dev/null
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.ts:3:export const useDropdownClose = ( /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.test.tsx:6:import { useDropdownClose } from './hooks' /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.test.tsx:10: useDropdownC …(truncated, 597 more chars)
I have a thorough understanding now. Let me compile the actionable findings.
Findings: Actionable simplification opportunities
I reviewed all changed files in branch fhir-2219_implement-multi-select-category-filter. Here are the actionable findings, ranked by impact.
1. MultiSelectDropdownPopup and MultiSelectDropdownOptions tests are functionally redundant (HIGH)
MultiSelectDropdownPopup.tsx is a 13-line wrapper that adds a <fieldset> + <legend> shell around MultiSelectDropdownOptions. Its test file (MultiSelectDropdownPopup.test.tsx, 129 lines) re-runs nearly every assertion already covered in MultiSelectDropdownOptions.test.tsx (100 lines) — options rendering, checked state, toggle interaction, and disabled options blocks are duplicates.
Only the describe('structure', ...) block (lines 32-50) is genuinely about the popup wrapper (dialog id, aria-label, hidden legend). Everything else is wrapping behavior that MultiSelectDropdownOptions already covers.
Recommendation: keep only the structure block in MultiSelectDropdownPopup.test.tsx and delete the options rendering, checked state, toggle interaction, and disabled options blocks (they remain covered by MultiSelectDropdownOptions.test.tsx). This drops ~80 lines of duplication without losing meaningful coverage — the popup's job is the dialog/fieldset shell, and the options' job is option rendering. The integration is already covered end-to-end by MultiSelectDropdown.test.tsx.
Files:
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownPopup.test.tsx
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownOptions.test.tsx
2. NhrDocumentCategoryFilter is a pure pass-through wrapper with no value (MEDIUM)
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentCategoryFilter/NhrDocumentCategoryFilter.tsx is now:
export type NhrDocumentCategoryFilterProps = ComponentProps<typeof MultiSelectDropdown>
export const NhrDocumentCategoryFilter = (props: NhrDocumentCategoryFilterProps) => <MultiSelectDropdown {...props} />
It adds zero behavior, zero typing constraint, zero default. The factory could import MultiSelectDropdown directly, eliminating the wrapper, the wrapper test (NhrDocumentCategoryFilter.test.tsx, 71 lines of duplicate coverage), the stories file, and the index.ts.
If the team genuinely wants a domain-named component (semantic indirection for future evolution), at minimum the test should be slimmed to one or two smoke tests — currently it duplicates MultiSelectDropdown.test.tsx cases verbatim. The factory test (createNhrDocumentCategoryFilterConfig.test.tsx) already exercises the integration.
Recommendation: delete the wrapper directory and replace the import in createNhrDocumentCategoryFilterConfig.tsx with import { MultiSelectDropdown } from '#components/MultiSelectDropdown'. If the wrapper is intentionally kept for naming, reduce the test to a single smoke test.
Files:
- /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/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx
3. displayLabel computation is overly clever (MEDIUM)
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdown.tsx lines 40-44:
const displayLabel = useMemo(() => {
const firstLabel = options.find((o) => values.includes(o.value))?.label ?? ''
if (values.length === 0 || firstLabel === '') return ''
return values.length === 1 ? firstLabel : `${firstLabel}, +${values.length - 1}`
}, [options, values])
The semantics are subtle: "the first option (in options order) whose value is selected". Tests like "should include unknown values in the +N count when they appear after a known value" prove the count includes unknown values, which is non-obvious from this code. Two improvements:
a) The values.length === 0 guard is redundant — when values is empty, find returns undefined and firstLabel === '' already returns ''. The combined guard reduces to if (firstLabel === '') return ''.
b) Add a one-line comment explaining the +N count includes ALL selected values (known or unknown), since this is non-obvious behavior under test.
Suggested:
const displayLabel = useMemo(() => {
// First option (in options order) whose value is selected; +N counts ALL
// selected values, including ones that don't match any option.
const firstLabel = options.find((o) => values.includes(o.value))?.label ?? ''
if (firstLabel === '') return ''
return values.length === 1 ? firstLabel : `${firstLabel}, +${values.length - 1}`
}, [options, values])
4. Unused parameter in factory render callback (LOW)
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/factories/filters/createNhrDocumentCategoryFilterConfig.tsx line 16:
render: (filters, onChange, _ctx) => {
The _ctx underscore prefix suggests it's unused, but this filter signature comes from a typed FilterConfig interface where ctx: FilterRenderContext. Keeping the underscore is fine, but it may trigger ESLint's no-unused-vars. Verify the lint config allows underscore-prefixed args; if not, this is a future foot-gun. (Most likely fine — Doctolib eslint usually allows ^_.)
Also, the toggle handler at lines 25-30 duplicates logic that the parent MultiSelectDropdown does not own — it could be a private helper for clarity:
const toggleValue = (list: string[], value: string): string[] =>
list.includes(value) ? list.filter((v) => v !== value) : [...list, value]
Then onToggle={(value) => onChange({ typeCodes: toggleValue(selectedValues, value) })}. Minor; acceptable as-is.
5. useDropdownClose ref-juggling is correct but adds noise (LOW)
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/hooks.ts keeps an onCloseRef to avoid re-attaching listeners when onClose changes. This is the correct React pattern, and there is a test (should call the latest onClose when the callback changes between renders) that locks in this behavior.
In MultiSelectDropdown.tsx, the close callback is recreated on every render anyway since it isn't memoized, so the ref pattern actively earns its keep here. No change recommended — flagging only because the comment ("Sync ref so the stable event listeners always call the latest onClose.") is one of the few comments in the codebase and is a good example of necessary explanation.
6. Trigger className concatenation could use clsx-style readability (LOW)
/Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/MultiSelectDropdown/MultiSelectDropdownTrigger.tsx line 38:
className={`oxygen-input-field__input ${isOpen ? 'cursor-text' : 'cursor-pointer'} ${hasValues ? 'pr-[5.5rem]' : 'pr-16'}`}
This is readable as-is and doesn't violate the "no nested ternary" rule. If the project uses clsx or classnames elsewhere in this package, it would slightly improve consistency. Not actionable without checking the package conventions; flagging for context only.
7. Storybook duplication between MultiSelectDropdown.stories.tsx an
…(truncated, 1433 more chars)