Skip to content

Commit 5b64091

Browse files
igorDykhtaigorDykhta
committed
fix: fixes to color by
Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
1 parent b6aee95 commit 5b64091

5 files changed

Lines changed: 95 additions & 23 deletions

File tree

‎src/components/src/common/color-legend.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export function LegendRowFactory(
168168
[color, onUpdateLabel]
169169
);
170170
const onReset = useCallback(() => onResetLabel && onResetLabel(color), [color, onResetLabel]);
171-
const value = displayLabel ? label.toString() : '';
171+
const value = displayLabel ? String(label ?? '') : '';
172172
return (
173173
<StyledLegendRow>
174174
<LegendColorDisplay color={color} />

‎src/components/src/side-panel/layer-panel/color-breaks-panel.tsx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
colorMapToColorBreaks,
1313
isNumericColorBreaks as notOrdinalColorBreaks
1414
} from '@kepler.gl/utils';
15-
import React, {useCallback, useMemo} from 'react';
15+
import React, {useCallback, useMemo, useEffect} from 'react';
1616
import styled from 'styled-components';
1717
import ColumnStatsChartFactory from '../../common/column-stats-chart';
1818
import {Edit} from '../../common/icons';
@@ -155,6 +155,16 @@ function ColorBreaksPanelFactory(
155155
[customPalette.colorMap, isEditingCustomBreaks, colorBreaks]
156156
);
157157

158+
// Update layers on editing custom breaks
159+
useEffect(() => {
160+
const {type} = customPalette || {};
161+
if (isEditingCustomBreaks) {
162+
if (type === SCALE_TYPES.customOrdinal || type === SCALE_TYPES.custom) {
163+
onScaleChange(type, customPalette);
164+
}
165+
}
166+
}, [isEditingCustomBreaks, customPalette, onScaleChange]);
167+
158168
const onClickEditCustomBreaks = useCallback(() => {
159169
setColorUI({
160170
colorRangeConfig: {
@@ -231,8 +241,8 @@ function ColorBreaksPanelFactory(
231241
currentBreaks={currentBreaks}
232242
onEdit={isCustomBreaks ? onClickEditCustomBreaks : null}
233243
/>
234-
) : customPalette.colorMap &&
235-
customPalette.type === 'customOrdinal' &&
244+
) : (isCustomBreaks || customPalette.type === SCALE_TYPES.customOrdinal) &&
245+
customPalette.colorMap &&
236246
customPalette.name?.endsWith(colorField.name) ? (
237247
<CategoricalColorDisplay
238248
colorMap={customPalette.colorMap}

‎src/components/src/side-panel/layer-panel/color-scale-selector.tsx‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,26 @@ function ColorScaleSelectorFactory(
148148
);
149149
const [tippyInstance, setTippyInstance] = useState<TippyInstance>();
150150
const isEditingColorBreaks = colorUIConfig?.colorRangeConfig?.customBreaks;
151+
152+
// Previous selection for live preview: when choosing Custom/Custom Ordinal we apply a temporary palette.
153+
// Cancel restores {scale, range} from this ref; Confirm keeps the change and clears the ref.
154+
const prevSelectionRef = React.useRef<{scale: string; range: ColorRange} | null>(null);
155+
156+
// when custom color scale - but Confirm is not clicked yet
157+
const pendingOption = useMemo(
158+
() =>
159+
isEditingColorBreaks
160+
? (dropdownSelectProps.options || []).find(
161+
o => getOptionValue(o) === colorUIConfig?.customPalette?.type
162+
) || null
163+
: null,
164+
[
165+
isEditingColorBreaks,
166+
dropdownSelectProps.options,
167+
getOptionValue,
168+
colorUIConfig?.customPalette?.type
169+
]
170+
);
151171
const colorScale = useMemo(
152172
() =>
153173
getLayerColorScale({
@@ -234,14 +254,16 @@ function ColorScaleSelectorFactory(
234254
const onSelectScale = useCallback(
235255
val => {
236256
// highlight selected option
237-
if (!val || isEditingColorBreaks) return;
257+
if (!val) return;
258+
238259
const selectedScale = getOptionValue(val);
239-
if (selectedScale === SCALE_TYPES.custom) {
260+
if (selectedScale === SCALE_TYPES.custom || selectedScale === SCALE_TYPES.customOrdinal) {
240261
const customPalette = initCustomPaletteByCustomScale({
241262
scale: selectedScale,
242263
field,
243264
range,
244-
colorBreaks
265+
colorBreaks,
266+
...(selectedScale === SCALE_TYPES.customOrdinal ? {ordinalDomain} : {})
245267
});
246268
setColorUI({
247269
showColorChart: true,
@@ -250,28 +272,53 @@ function ColorScaleSelectorFactory(
250272
},
251273
customPalette
252274
});
275+
// store previous selection for cancel, then preview custom on the map
276+
prevSelectionRef.current = {scale: scaleType, range};
253277
onSelect(selectedScale, customPalette);
254-
} else if (hasColorMap(range) && selectedScale !== SCALE_TYPES.customOrdinal) {
278+
} else if (hasColorMap(range)) {
255279
// not custom
256280
// remove colorMap
257281
// eslint-disable-next-line no-unused-vars
258282
const {colorMap: _, ...newRange} = range;
283+
// reset colorUI before changing the scale
284+
setColorUI({
285+
showColorChart: false,
286+
colorRangeConfig: {
287+
customBreaks: false
288+
}
289+
});
259290
onSelect(selectedScale, newRange);
260291
} else {
292+
// reset colorUI before changing the scale
293+
setColorUI({
294+
showColorChart: false,
295+
colorRangeConfig: {
296+
customBreaks: false
297+
}
298+
});
261299
onSelect(selectedScale);
262300
}
263301
},
264-
[isEditingColorBreaks, field, setColorUI, onSelect, range, getOptionValue, colorBreaks]
302+
[field, setColorUI, onSelect, range, getOptionValue, colorBreaks, ordinalDomain, scaleType]
265303
);
266304

267305
const onApply = useCallback(() => {
268-
onSelect(scaleType, colorUIConfig.customPalette);
306+
// change scale type only if confirmed
307+
const nextScaleType = colorUIConfig?.customPalette?.type || scaleType;
308+
onSelect(nextScaleType, colorUIConfig.customPalette);
269309
hideTippy(tippyInstance);
310+
prevSelectionRef.current = null;
270311
}, [onSelect, colorUIConfig.customPalette, tippyInstance, scaleType]);
271312

272313
const onCancel = useCallback(() => {
314+
// restore previous selection if any
315+
if (prevSelectionRef.current) {
316+
const {scale: prevScale, range: prevRange} = prevSelectionRef.current;
317+
onSelect(prevScale, prevRange);
318+
}
273319
hideTippy(tippyInstance);
274-
}, [tippyInstance]);
320+
prevSelectionRef.current = null;
321+
}, [tippyInstance, onSelect]);
275322

276323
const isCustomBreaks =
277324
scaleType === SCALE_TYPES.custom || scaleType === SCALE_TYPES.customOrdinal;
@@ -317,6 +364,9 @@ function ColorScaleSelectorFactory(
317364
customListComponent={ColorScaleSelectDropdown}
318365
searchable={false}
319366
showOptionsWhenEmpty
367+
selectedItems={
368+
pendingOption ? [pendingOption] : dropdownSelectProps.selectedItems
369+
}
320370
/>
321371
)}
322372
</DropdownWrapper>
@@ -327,7 +377,7 @@ function ColorScaleSelectorFactory(
327377
<DropdownSelect
328378
{...dropdownSelectProps}
329379
displayOption={displayOption}
330-
value={dropdownSelectProps.selectedItems[0]}
380+
value={pendingOption || dropdownSelectProps.selectedItems[0]}
331381
/>
332382
</div>
333383
</LazyTippy>

‎src/components/src/side-panel/layer-panel/custom-palette.tsx‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,11 @@ export const EditableColorRange: React.FC<EditableColorRangeProps> = ({
377377
editColorMap,
378378
editable
379379
}) => {
380-
const noMinBound = !Number.isFinite(item.inputs[0]) && index === 0;
381-
const noMaxBound = !Number.isFinite(item.inputs[1]) && isLast;
380+
const hasInputs = Array.isArray(item?.inputs);
381+
const leftInput = hasInputs ? item.inputs[0] : undefined;
382+
const rightInput = hasInputs ? item.inputs[1] : undefined;
383+
const noMinBound = !Number.isFinite(leftInput) && index === 0;
384+
const noMaxBound = !Number.isFinite(rightInput) && isLast;
382385
const onChangeLeft = useCallback(
383386
val => {
384387
if (editable && editColorMap) editColorMap(parseFloat(val), index - 1);
@@ -395,7 +398,7 @@ export const EditableColorRange: React.FC<EditableColorRangeProps> = ({
395398
return (
396399
<StyledRangeInput>
397400
<ColorPaletteInput
398-
value={noMinBound ? 'Less' : item.inputs[0].toString()}
401+
value={noMinBound ? 'Less' : String(leftInput ?? '')}
399402
id={`color-palette-input-${index}-left`}
400403
width="50px"
401404
textAlign="end"
@@ -404,7 +407,7 @@ export const EditableColorRange: React.FC<EditableColorRangeProps> = ({
404407
/>
405408
<Dash />
406409
<ColorPaletteInput
407-
value={noMaxBound ? 'More' : item.inputs[1].toString()}
410+
value={noMaxBound ? 'More' : String(rightInput ?? '')}
408411
id={`color-palette-input-${index}-right`}
409412
width="50px"
410413
textAlign="end"
@@ -468,7 +471,7 @@ export const CustomPaletteInput: React.FC<CustomPaletteInputProps> = ({
468471
/>
469472
</StyledColorHexInput>
470473
) : null}
471-
{isNumericColorBreaks(colorBreaks) ? (
474+
{isNumericColorBreaks(colorBreaks) && colorBreaks && index < colorBreaks.length ? (
472475
<EditableColorRange
473476
item={colorBreaks[index]}
474477
isLast={index === colorBreaks.length - 1}
@@ -719,6 +722,11 @@ export const CategoricalSelector: React.FC<CategoricalSelectorProps> = ({
719722
listAnchor: 'list__item__anchor'
720723
}}
721724
options={allValues}
725+
// add safe string casting for the Typeahead, so fuzzy search never receives non-strings, preventing the toLowerCase crash
726+
displayOption={o => String(o ?? '')}
727+
filterOption={(input, o) =>
728+
String(o ?? '').includes(String(input ?? '').toLowerCase())
729+
}
722730
placeholder={'Search'}
723731
onOptionSelected={onOptionSelected}
724732
customListComponent={ModifiedDropdownList}

‎test/browser/components/side-panel/channel-by-value-selctor-test.js‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,11 @@ test('Components -> ChannelByValueSelector -> ColorScaleSelector -> ColorBreakDi
373373
t.equal(confirmButton.text(), 'Confirm', 'should render confirm button');
374374
confirmButton.simulate('click');
375375

376-
t.ok(updateLayerVisualChannelConfig.calledTwice, 'should call updateLayerVisualChannelConfig');
376+
// Confirm should result in at least a second update call
377+
t.ok(
378+
updateLayerVisualChannelConfig.args && updateLayerVisualChannelConfig.args.length >= 2,
379+
'should call updateLayerVisualChannelConfig'
380+
);
377381

378382
const expectedArgs3 = [
379383
{colorScale: 'custom'},
@@ -432,13 +436,13 @@ test('Components -> ChannelByValueSelector -> ColorScaleSelector -> ColorBreakDi
432436
quantileOption.simulate('click');
433437

434438
const expectedArgs5 = [
435-
{colorScale: 'quantile'},
439+
{colorScale: 'quantize'},
436440
'color',
437441
{
438442
colorRange: {
439-
name: 'color.customPalette.custom.uid',
440-
type: 'custom',
441-
category: 'Custom',
443+
name: 'Uber Viz Sequential',
444+
type: 'sequential',
445+
category: 'Uber',
442446
colors: ['#00939C', '#6BB5B9', '#AAD7D9', '#E6FAFA']
443447
}
444448
}
@@ -450,7 +454,7 @@ test('Components -> ChannelByValueSelector -> ColorScaleSelector -> ColorBreakDi
450454
'should pass custom scale and custom colorRange'
451455
);
452456

453-
t.deepEqual(setColorUI.args.length, 3, 'should not call setColorUI');
457+
t.deepEqual(setColorUI.args.length, 4, 'should not call setColorUI');
454458

455459
t.end();
456460
});

0 commit comments

Comments
 (0)