-
Notifications
You must be signed in to change notification settings - Fork 138
Added Colorblind-Accessible Color Palettes and Opacity Control for Isochrones #344
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,17 +25,32 @@ import { AccessibleIcon } from '@radix-ui/react-accessible-icon'; | |||||||||||
| import { parseUrlParams } from '@/utils/parse-url-params'; | ||||||||||||
| import { useNavigate } from '@tanstack/react-router'; | ||||||||||||
| import { useIsochronesQuery } from '@/hooks/use-isochrones-queries'; | ||||||||||||
| import { | ||||||||||||
| Select, | ||||||||||||
| SelectContent, | ||||||||||||
| SelectItem, | ||||||||||||
| SelectTrigger, | ||||||||||||
| SelectValue, | ||||||||||||
| } from '@/components/ui/select'; | ||||||||||||
| import { Label } from '@/components/ui/label'; | ||||||||||||
| import { ISOCHRONE_PALETTES } from '@/utils/isochrone-colors'; | ||||||||||||
|
|
||||||||||||
| export const Waypoints = () => { | ||||||||||||
| const params = parseUrlParams(); | ||||||||||||
| const updateSettings = useIsochronesStore((state) => state.updateSettings); | ||||||||||||
| const updateColorPalette = useIsochronesStore( | ||||||||||||
| (state) => state.updateColorPalette | ||||||||||||
| ); | ||||||||||||
| const updateOpacity = useIsochronesStore((state) => state.updateOpacity); | ||||||||||||
| const { refetch: refetchIsochrones } = useIsochronesQuery(); | ||||||||||||
| const clearIsos = useIsochronesStore((state) => state.clearIsos); | ||||||||||||
| const updateTextInput = useIsochronesStore((state) => state.updateTextInput); | ||||||||||||
| const maxRange = useIsochronesStore((state) => state.maxRange); | ||||||||||||
| const interval = useIsochronesStore((state) => state.interval); | ||||||||||||
| const denoise = useIsochronesStore((state) => state.denoise); | ||||||||||||
| const generalize = useIsochronesStore((state) => state.generalize); | ||||||||||||
| const colorPalette = useIsochronesStore((state) => state.colorPalette); | ||||||||||||
| const opacity = useIsochronesStore((state) => state.opacity); | ||||||||||||
| const navigate = useNavigate({ from: '/$activeTab' }); | ||||||||||||
| const userInput = useIsochronesStore((state) => state.userInput); | ||||||||||||
| const geocodeResults = useIsochronesStore((state) => state.geocodeResults); | ||||||||||||
|
|
@@ -237,6 +252,42 @@ export const Waypoints = () => { | |||||||||||
| makeIsochronesRequestDebounced(); | ||||||||||||
| }} | ||||||||||||
| /> | ||||||||||||
|
|
||||||||||||
| <div className="flex flex-col gap-2"> | ||||||||||||
| <Label htmlFor="color-palette" className="text-sm font-medium"> | ||||||||||||
| Color Palette | ||||||||||||
| </Label> | ||||||||||||
| <Select value={colorPalette} onValueChange={updateColorPalette}> | ||||||||||||
|
||||||||||||
| <Select value={colorPalette} onValueChange={updateColorPalette}> | |
| <Select | |
| value={colorPalette} | |
| onValueChange={(value) => updateColorPalette(value as IsochronePalette)} | |
| > |
Copilot
AI
Mar 5, 2026
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.
There's already a SelectSetting component (src/components/ui/select-setting.tsx) that wraps a Select with a Label, description tooltip (with a help icon), and consistent styling — matching the pattern used by SliderSetting for other settings in this same panel. Consider using SelectSetting here instead of manually assembling Label, Select, SelectTrigger, etc. This would provide a consistent UI (including a description/help tooltip for the color palette setting) and reduce inline code. The ISOCHRONE_PALETTES entries would need to be mapped to SelectOption format ({ key, text, value }).
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,13 +42,16 @@ const createMockState = (overrides = {}) => ({ | |||||||||||||
| ], | ||||||||||||||
| ], | ||||||||||||||
| }, | ||||||||||||||
| properties: { fill: '#ff0000' }, | ||||||||||||||
| properties: { fill: '#ff0000', contour: 10 }, | ||||||||||||||
| }, | ||||||||||||||
| ], | ||||||||||||||
| }, | ||||||||||||||
| show: true, | ||||||||||||||
| }, | ||||||||||||||
| successful: true, | ||||||||||||||
| colorPalette: 'current', | ||||||||||||||
| opacity: 0.4, | ||||||||||||||
| maxRange: 10, | ||||||||||||||
|
Comment on lines
+52
to
+54
|
||||||||||||||
| colorPalette: 'current', | |
| opacity: 0.4, | |
| maxRange: 10, | |
| colorPalette: 'current', | |
| opacity: 0.4, | |
| maxRange: 10, |
Copilot
AI
Mar 5, 2026
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 isochrone-polygons tests only exercise the 'current' palette. There's no test verifying that the fillColor property on features is computed correctly when using 'viridis' or 'magma' palettes (i.e., that getIsochroneColor is called with the correct normalized value). Consider adding a test case that uses a non-default colorPalette and verifies the resulting feature colors in the Source data.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,10 +2,14 @@ import { useMemo } from 'react'; | |||||
| import { Source, Layer } from 'react-map-gl/maplibre'; | ||||||
| import { useIsochronesStore } from '@/stores/isochrones-store'; | ||||||
| import type { Feature, FeatureCollection } from 'geojson'; | ||||||
| import { getIsochroneColor } from '@/utils/isochrone-colors'; | ||||||
|
|
||||||
| export function IsochronePolygons() { | ||||||
| const isoResults = useIsochronesStore((state) => state.results); | ||||||
| const isoSuccessful = useIsochronesStore((state) => state.successful); | ||||||
| const colorPalette = useIsochronesStore((state) => state.colorPalette); | ||||||
| const opacity = useIsochronesStore((state) => state.opacity); | ||||||
| const maxRange = useIsochronesStore((state) => state.maxRange); | ||||||
|
|
||||||
| const data = useMemo(() => { | ||||||
| if (!isoResults || !isoSuccessful) return null; | ||||||
|
|
@@ -18,11 +22,22 @@ export function IsochronePolygons() { | |||||
|
|
||||||
| for (const feature of isoResults.data.features) { | ||||||
| if (['Polygon', 'MultiPolygon'].includes(feature.geometry.type)) { | ||||||
| const contourValue = feature.properties?.contour ?? maxRange; | ||||||
| let fillColor: string; | ||||||
|
|
||||||
| if (colorPalette === 'current') { | ||||||
| fillColor = feature.properties?.fill || '#6200ea'; | ||||||
| } else { | ||||||
| const normalizedValue = contourValue / maxRange; | ||||||
|
||||||
| const normalizedValue = contourValue / maxRange; | |
| const normalizedValue = maxRange > 0 ? contourValue / maxRange : 0; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { getIsochroneColor, ISOCHRONE_PALETTES } from './isochrone-colors'; | ||
|
|
||
| describe('isochrone-colors', () => { | ||
| describe('getIsochroneColor', () => { | ||
| it('should return a valid hex color for any value between 0 and 1', () => { | ||
| const testValues = [0, 0.25, 0.5, 0.75, 1.0]; | ||
| for (const value of testValues) { | ||
| const color = getIsochroneColor(value, 'viridis'); | ||
| expect(color).toMatch(/^#[0-9a-f]{6}$/i); | ||
| } | ||
| }); | ||
|
|
||
| it('should handle values outside [0, 1] by clamping them', () => { | ||
| const colorNegative = getIsochroneColor(-0.5, 'viridis'); | ||
| const colorZero = getIsochroneColor(0, 'viridis'); | ||
| const colorOver = getIsochroneColor(1.5, 'viridis'); | ||
| const colorOne = getIsochroneColor(1.0, 'viridis'); | ||
|
|
||
| expect(colorNegative).toBe(colorZero); | ||
|
|
||
| expect(colorOver).toBe(colorOne); | ||
| }); | ||
|
|
||
| describe('viridis palette', () => { | ||
| it('should start with dark purple at value 0', () => { | ||
| const color = getIsochroneColor(0, 'viridis'); | ||
|
|
||
| expect(color).toMatch(/^#[0-9a-f]{6}$/i); | ||
| }); | ||
|
|
||
| it('should end with yellow at value 1.0', () => { | ||
| const color = getIsochroneColor(1.0, 'viridis'); | ||
| expect(color).toMatch(/^#[0-9a-f]{6}$/i); | ||
| }); | ||
|
|
||
| it('should return different colors across the range', () => { | ||
| const colors = [0, 0.25, 0.5, 0.75, 1.0].map((v) => | ||
| getIsochroneColor(v, 'viridis') | ||
| ); | ||
| const uniqueColors = new Set(colors); | ||
| expect(uniqueColors.size).toBe(colors.length); | ||
| }); | ||
|
|
||
| it('should have smooth interpolation (no flat regions)', () => { | ||
| const color74 = getIsochroneColor(0.74, 'viridis'); | ||
| const color76 = getIsochroneColor(0.76, 'viridis'); | ||
| expect(color74).not.toBe(color76); | ||
| }); | ||
| }); | ||
|
|
||
| describe('current palette', () => { | ||
| it('should return colors for all values', () => { | ||
| const testValues = [0, 0.25, 0.5, 0.75, 1.0]; | ||
| for (const value of testValues) { | ||
| const color = getIsochroneColor(value, 'current'); | ||
| expect(color).toMatch(/^#[0-9a-f]{6}$/i); | ||
| } | ||
| }); | ||
|
|
||
| it('should return different colors across the range', () => { | ||
| const colors = [0, 0.33, 0.66, 1.0].map((v) => | ||
| getIsochroneColor(v, 'current') | ||
| ); | ||
| const uniqueColors = new Set(colors); | ||
| expect(uniqueColors.size).toBeGreaterThan(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe('magma palette', () => { | ||
| it('should return colors for all values', () => { | ||
| const testValues = [0, 0.25, 0.5, 0.75, 1.0]; | ||
| for (const value of testValues) { | ||
| const color = getIsochroneColor(value, 'magma'); | ||
| expect(color).toMatch(/^#[0-9a-f]{6}$/i); | ||
| } | ||
| }); | ||
|
|
||
| it('should return different colors across the range', () => { | ||
| const colors = [0, 0.25, 0.5, 0.75, 1.0].map((v) => | ||
| getIsochroneColor(v, 'magma') | ||
| ); | ||
| const uniqueColors = new Set(colors); | ||
| expect(uniqueColors.size).toBe(colors.length); | ||
| }); | ||
| }); | ||
|
|
||
| it('should use current palette as default', () => { | ||
| const colorDefault = getIsochroneColor(0.5); | ||
| const colorExplicit = getIsochroneColor(0.5, 'current'); | ||
| expect(colorDefault).toBe(colorExplicit); | ||
| }); | ||
|
|
||
| it('should return current palette for invalid palette name', () => { | ||
| const colorInvalid = getIsochroneColor(0.5, 'invalid' as any); | ||
| const colorCurrent = getIsochroneColor(0.5, 'current'); | ||
| expect(colorInvalid).toBe(colorCurrent); | ||
| }); | ||
|
|
||
| it('should return different colors for different palettes', () => { | ||
| const color1 = getIsochroneColor(0.5, 'viridis'); | ||
| const color2 = getIsochroneColor(0.5, 'magma'); | ||
| const color3 = getIsochroneColor(0.5, 'current'); | ||
|
|
||
| // At least some should be different | ||
| const uniqueColors = new Set([color1, color2, color3]); | ||
| expect(uniqueColors.size).toBeGreaterThan(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe('ISOCHRONE_PALETTES', () => { | ||
| it('should define all required palettes', () => { | ||
| expect(ISOCHRONE_PALETTES).toHaveProperty('viridis'); | ||
| expect(ISOCHRONE_PALETTES).toHaveProperty('magma'); | ||
| expect(ISOCHRONE_PALETTES).toHaveProperty('current'); | ||
| }); | ||
|
|
||
| it('should have correct structure for each palette', () => { | ||
| Object.entries(ISOCHRONE_PALETTES).forEach(([key, palette]) => { | ||
| expect(palette).toHaveProperty('label'); | ||
| expect(palette).toHaveProperty('value'); | ||
| expect(typeof palette.label).toBe('string'); | ||
| expect(palette.value).toBe(key); | ||
| }); | ||
| }); | ||
|
|
||
| it('should have non-empty labels', () => { | ||
| Object.values(ISOCHRONE_PALETTES).forEach((palette) => { | ||
| expect(palette.label.length).toBeGreaterThan(0); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
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 mocks for
updateColorPaletteandupdateOpacityare added, but there are no corresponding test cases that verify the Color Palette selector and Opacity slider are rendered in the settings panel or that interactions with them invoke the correct store actions. The existing tests verify rendering and interaction for all other settings (maxRange, interval, denoise, generalize). Consider adding similar tests for the new controls to maintain consistent coverage.