-
Notifications
You must be signed in to change notification settings - Fork 31
fix: show full geo name behavior on county maps #2706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| type AddOptionalFullGeoNameColumnArgs = { | ||
| config: any | ||
| csvData: Record<string, any>[] | ||
| csvDataUpdated: Record<string, any>[] | ||
| formatLegendLocation?: (key: string) => string | ||
| } | ||
|
|
||
| export const addOptionalFullGeoNameColumn = ({ | ||
| config, | ||
| csvData, | ||
| csvDataUpdated, | ||
| formatLegendLocation | ||
| }: AddOptionalFullGeoNameColumnArgs) => { | ||
| if (!config?.table?.showFullGeoNameInCSV || typeof formatLegendLocation !== 'function') { | ||
| return csvDataUpdated | ||
| } | ||
|
|
||
| return csvDataUpdated.map((row, index) => { | ||
| const originalRow = csvData[index] | ||
| if (!originalRow) { | ||
| console.warn('Data mismatch: originalRow missing.', { | ||
| index, | ||
| csvDataLength: csvData.length, | ||
| csvDataUpdatedLength: csvDataUpdated.length | ||
| }) | ||
| return row | ||
| } | ||
|
|
||
| return { | ||
| FullGeoName: formatLegendLocation(originalRow[config.columns.geo.name]), | ||
| ...row | ||
|
Comment on lines
+14
to
+31
|
||
| } | ||
| }) | ||
| } | ||
|
|
||
| export default addOptionalFullGeoNameColumn | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { describe, expect, it, vi } from 'vitest' | ||
| import { addOptionalFullGeoNameColumn } from '../addOptionalFullGeoNameColumn' | ||
|
|
||
| describe('addOptionalFullGeoNameColumn', () => { | ||
| const csvData = [{ geo: '01001', value: 1 }] | ||
| const csvDataUpdated = [{ Geo: '01001', Value: 1 }] | ||
| const formatLegendLocation = vi.fn((key: string) => `AL, ${key}`) | ||
|
|
||
| it('does not add FullGeoName for county maps when the toggle is off', () => { | ||
| const result = addOptionalFullGeoNameColumn({ | ||
| config: { | ||
| general: { geoType: 'us-county' }, | ||
| table: { showFullGeoNameInCSV: false }, | ||
| columns: { geo: { name: 'geo' } } | ||
| }, | ||
| csvData, | ||
| csvDataUpdated, | ||
| formatLegendLocation | ||
| }) | ||
|
|
||
| expect(result).toEqual(csvDataUpdated) | ||
| expect(result[0]).not.toHaveProperty('FullGeoName') | ||
| }) | ||
|
|
||
| it('adds FullGeoName for county maps when the toggle is on', () => { | ||
| const result = addOptionalFullGeoNameColumn({ | ||
| config: { | ||
| general: { geoType: 'us-county' }, | ||
| table: { showFullGeoNameInCSV: true }, | ||
| columns: { geo: { name: 'geo' } } | ||
| }, | ||
| csvData, | ||
| csvDataUpdated, | ||
| formatLegendLocation | ||
| }) | ||
|
|
||
| expect(result).toEqual([{ FullGeoName: 'AL, 01001', Geo: '01001', Value: 1 }]) | ||
| }) | ||
|
|
||
| it('does not add FullGeoName for non-county maps when the toggle is off', () => { | ||
| const result = addOptionalFullGeoNameColumn({ | ||
| config: { | ||
| general: { geoType: 'us' }, | ||
| table: { showFullGeoNameInCSV: false }, | ||
| columns: { geo: { name: 'geo' } } | ||
| }, | ||
| csvData, | ||
| csvDataUpdated, | ||
| formatLegendLocation | ||
| }) | ||
|
|
||
| expect(result).toEqual(csvDataUpdated) | ||
| }) | ||
|
|
||
| it('adds FullGeoName for non-county maps when the toggle is on', () => { | ||
| const result = addOptionalFullGeoNameColumn({ | ||
| config: { | ||
| general: { geoType: 'us' }, | ||
| table: { showFullGeoNameInCSV: true }, | ||
| columns: { geo: { name: 'geo' } } | ||
| }, | ||
| csvData, | ||
| csvDataUpdated, | ||
| formatLegendLocation | ||
| }) | ||
|
|
||
| expect(result).toEqual([{ FullGeoName: 'AL, 01001', Geo: '01001', Value: 1 }]) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
formatLegendLocationtype here is(key: string) => string, butDataTableProps.formatLegendLocationis typed as(row: string, runtimeLookup: string) => string. To keep types consistent and avoid subtle TS assignability issues, consider reusing the prop type (e.g.,DataTableProps['formatLegendLocation']) or updating this signature to accept the optional second parameter.