Skip to content

Commit 2452965

Browse files
igorDykhtaigorDykhta
authored
fix: fixes to channel by value (#3216)
* fix: fixes to color by Signed-off-by: Ihor Dykhta <[email protected]> * fix cancel Signed-off-by: Ihor Dykhta <[email protected]> * nit Signed-off-by: Ihor Dykhta <[email protected]> * nit Signed-off-by: Ihor Dykhta <[email protected]> * nit Signed-off-by: Ihor Dykhta <[email protected]> --------- Signed-off-by: Ihor Dykhta <[email protected]>
1 parent f211ccd commit 2452965

File tree

5 files changed

+98
-23
lines changed

5 files changed

+98
-23
lines changed

‎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: 14 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,17 @@ function ColorBreaksPanelFactory(
155155
[customPalette.colorMap, isEditingCustomBreaks, colorBreaks]
156156
);
157157

158+
// Update layers on editing custom breaks
159+
useEffect(() => {
160+
const {type} = customPalette || {};
161+
if (
162+
isEditingCustomBreaks &&
163+
(type === SCALE_TYPES.customOrdinal || type === SCALE_TYPES.custom)
164+
) {
165+
onScaleChange(type, customPalette);
166+
}
167+
}, [isEditingCustomBreaks, customPalette, onScaleChange]);
168+
158169
const onClickEditCustomBreaks = useCallback(() => {
159170
setColorUI({
160171
colorRangeConfig: {
@@ -231,8 +242,8 @@ function ColorBreaksPanelFactory(
231242
currentBreaks={currentBreaks}
232243
onEdit={isCustomBreaks ? onClickEditCustomBreaks : null}
233244
/>
234-
) : customPalette.colorMap &&
235-
customPalette.type === 'customOrdinal' &&
245+
) : (isCustomBreaks || customPalette.type === SCALE_TYPES.customOrdinal) &&
246+
customPalette.colorMap &&
236247
customPalette.name?.endsWith(colorField.name) ? (
237248
<CategoricalColorDisplay
238249
colorMap={customPalette.colorMap}

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

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,28 @@ function ColorScaleSelectorFactory(
148148
);
149149
const [tippyInstance, setTippyInstance] = useState<TippyInstance>();
150150
const isEditingColorBreaks = colorUIConfig?.colorRangeConfig?.customBreaks;
151+
152+
// Stores the 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+
// If the user switches between different custom scale types (e.g., from "Custom" to "Custom Ordinal") or is already in a custom scale state,
155+
// this ref is updated to always store the most recent non-custom selection. Only the latest non-custom selection is restorable on cancel.
156+
const prevSelectionRef = React.useRef<{scale: string; range: ColorRange} | null>(null);
157+
158+
// when custom color scale - but Confirm is not clicked yet
159+
const pendingOption = useMemo(
160+
() =>
161+
isEditingColorBreaks
162+
? (dropdownSelectProps.options || []).find(
163+
o => getOptionValue(o) === colorUIConfig?.customPalette?.type
164+
) || null
165+
: null,
166+
[
167+
isEditingColorBreaks,
168+
dropdownSelectProps.options,
169+
getOptionValue,
170+
colorUIConfig?.customPalette?.type
171+
]
172+
);
151173
const colorScale = useMemo(
152174
() =>
153175
getLayerColorScale({
@@ -234,14 +256,16 @@ function ColorScaleSelectorFactory(
234256
const onSelectScale = useCallback(
235257
val => {
236258
// highlight selected option
237-
if (!val || isEditingColorBreaks) return;
259+
if (!val) return;
260+
238261
const selectedScale = getOptionValue(val);
239-
if (selectedScale === SCALE_TYPES.custom) {
262+
if (selectedScale === SCALE_TYPES.custom || selectedScale === SCALE_TYPES.customOrdinal) {
240263
const customPalette = initCustomPaletteByCustomScale({
241264
scale: selectedScale,
242265
field,
243266
range,
244-
colorBreaks
267+
colorBreaks,
268+
...(selectedScale === SCALE_TYPES.customOrdinal ? {ordinalDomain} : {})
245269
});
246270
setColorUI({
247271
showColorChart: true,
@@ -250,28 +274,55 @@ function ColorScaleSelectorFactory(
250274
},
251275
customPalette
252276
});
277+
// store previous selection for cancel, then preview custom on the map
278+
if (!prevSelectionRef.current) {
279+
prevSelectionRef.current = {scale: scaleType, range};
280+
}
253281
onSelect(selectedScale, customPalette);
254-
} else if (hasColorMap(range) && selectedScale !== SCALE_TYPES.customOrdinal) {
282+
} else if (hasColorMap(range)) {
255283
// not custom
256284
// remove colorMap
257285
// eslint-disable-next-line no-unused-vars
258286
const {colorMap: _, ...newRange} = range;
287+
// reset colorUI before changing the scale
288+
setColorUI({
289+
showColorChart: false,
290+
colorRangeConfig: {
291+
customBreaks: false
292+
}
293+
});
259294
onSelect(selectedScale, newRange);
260295
} else {
296+
// reset colorUI before changing the scale
297+
setColorUI({
298+
showColorChart: false,
299+
colorRangeConfig: {
300+
customBreaks: false
301+
}
302+
});
261303
onSelect(selectedScale);
262304
}
263305
},
264-
[isEditingColorBreaks, field, setColorUI, onSelect, range, getOptionValue, colorBreaks]
306+
[field, setColorUI, onSelect, range, getOptionValue, colorBreaks, ordinalDomain, scaleType]
265307
);
266308

267309
const onApply = useCallback(() => {
268-
onSelect(scaleType, colorUIConfig.customPalette);
310+
// change scale type only if confirmed
311+
const nextScaleType = colorUIConfig?.customPalette?.type || scaleType;
312+
onSelect(nextScaleType, colorUIConfig.customPalette);
269313
hideTippy(tippyInstance);
314+
prevSelectionRef.current = null;
270315
}, [onSelect, colorUIConfig.customPalette, tippyInstance, scaleType]);
271316

272317
const onCancel = useCallback(() => {
318+
// restore previous selection if any
319+
if (prevSelectionRef.current) {
320+
const {scale: prevScale, range: prevRange} = prevSelectionRef.current;
321+
onSelect(prevScale, prevRange);
322+
}
273323
hideTippy(tippyInstance);
274-
}, [tippyInstance]);
324+
prevSelectionRef.current = null;
325+
}, [tippyInstance, onSelect]);
275326

276327
const isCustomBreaks =
277328
scaleType === SCALE_TYPES.custom || scaleType === SCALE_TYPES.customOrdinal;
@@ -317,6 +368,9 @@ function ColorScaleSelectorFactory(
317368
customListComponent={ColorScaleSelectDropdown}
318369
searchable={false}
319370
showOptionsWhenEmpty
371+
selectedItems={
372+
pendingOption ? [pendingOption] : dropdownSelectProps.selectedItems
373+
}
320374
/>
321375
)}
322376
</DropdownWrapper>
@@ -327,7 +381,7 @@ function ColorScaleSelectorFactory(
327381
<DropdownSelect
328382
{...dropdownSelectProps}
329383
displayOption={displayOption}
330-
value={dropdownSelectProps.selectedItems[0]}
384+
value={pendingOption || dropdownSelectProps.selectedItems[0]}
331385
/>
332386
</div>
333387
</LazyTippy>

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

Lines changed: 11 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+
{colorBreaks && index < colorBreaks.length && isNumericColorBreaks(colorBreaks) ? (
472475
<EditableColorRange
473476
item={colorBreaks[index]}
474477
isLast={index === colorBreaks.length - 1}
@@ -719,6 +722,9 @@ 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) => String(o ?? '').includes(String(input ?? ''))}
722728
placeholder={'Search'}
723729
onOptionSelected={onOptionSelected}
724730
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)