fix: resolve CI failures in TypeScript migration#47
Conversation
- Fix prettier formatting for all migrated .tsx/.ts files - Fix ESLint errors: remove unused eslint-disable directives, replace any types - Fix dropdown-button test: update modal import from .js to .tsx - Fix simple-single-select-field: restore missing labelId prop on Field - Fix table-header-cell/table-data-cell tests: expect Number() for colSpan/rowSpan - Fix organisation-unit-tree tests: replace obsolete prop-types tests with TS-compatible tests - Fix broken story imports: update .js/.tsx references to match renamed files - Fix scripts/ts-check.js formatting Co-Authored-By: Eirik <eirik.haugstulen@gmail.com>
|
Prompt hidden (unlisted session) |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| disabled={disabled} | ||
| required={required} | ||
| name={name} | ||
| labelId={labelId} |
There was a problem hiding this comment.
🔴 SimpleSingleSelectField drops name prop from Field, breaking label's htmlFor attribute
The change on line 90 replaces name={name} with labelId={labelId} when rendering the <Field> component. While this correctly adds an id to the <label> element (enabling aria-labelledby on the select), it removes the name prop that Field uses to set htmlFor on the label (components/field/src/field/field.tsx:54). Without htmlFor, clicking the label no longer focuses the associated select input, which is an accessibility regression. Both name={name} and labelId={labelId} should be passed to Field.
| labelId={labelId} | |
| labelId={labelId} | |
| name={name} |
Was this helpful? React with 👍 or 👎 to provide feedback.
| }: FieldGroupProps) => ( | ||
| <FieldSet {...({ classname: className } as Record<string, unknown>)} dataTest={dataTest}> | ||
| <FieldSet | ||
| {...({ classname: className } as Record<string, unknown>)} |
There was a problem hiding this comment.
🔴 FieldGroup passes classname (lowercase) instead of className to FieldSet, so CSS class is never applied
The spread object uses classname (lowercase 'n') instead of className (camelCase): { classname: className }. Since React prop names are case-sensitive, this results in an unrecognized DOM attribute classname being passed instead of the standard className. The className prop value provided to FieldGroup is silently discarded and never applied to the <FieldSet> element. This is a pre-existing bug that is visible in the reformatted lines.
| {...({ classname: className } as Record<string, unknown>)} | |
| {...({ className: className } as Record<string, unknown>)} |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Fixes CI failures from dhis2#1736
Description
This PR addresses all CI failures found in the TypeScript migration branch (
eh/feat/rewrite-to-typescript):Prettier formatting
.tsx/.tsfiles andscripts/ts-check.jsESLint errors
eslint-disabledirectives inuse-root-org-data.tsandtransfer-option.tsxRecord<string, any>with proper typed alternatives inuse-root-org-data.tsTest fixes
dropdown-button.test.js: Updated modal import from.jsto.tsx(file was renamed during migration)simple-single-select-field.tsx: Restored missinglabelIdprop onFieldcomponent (was dropped during migration)table-header-cell.test.js/table-data-cell.test.js: Updated colSpan/rowSpan assertions to expectNumber()values (component now converts string to number)organisation-unit-tree.test.js: Replaced obsolete prop-types validation tests with TypeScript-compatible render tests (prop-types no longer fire with TS interfaces)Broken story imports
../simple-single-select.js→.tsx./index.tsx→./index.ts./divider.js→./divider.tsxKnown issues
Checklist
Screenshots
N/A - these are lint/test/formatting fixes only
Link to Devin session: https://app.devin.ai/sessions/75b6a28c77ad4d729c55d304516932ac
Requested by: @eirikhaugstulen