Jan26 qa fixes #277

Arbyhisenaj · 2026-01-20T12:12:07Z

Description

Motivation and Context

How Can It Be Tested?

How Will This Be Deployed?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I've checked the spec (e.g. Figma file) and documented any divergences.
My code follows the code style of this project.
My code follows the code style of this project.
My change requires a change to the documentation.
I've updated the documentation accordingly.
Replace unused checkboxes with bullet points.

Copilot

Pull request overview

This PR contains QA fixes for the visualization panel, focusing on UI improvements and stepped color editor logic refinements for handling data source and column changes.

Changes:

Updated grid layouts and select components with better minimum width constraints to prevent overflow issues
Added comprehensive logic to reset and validate stepped color steps when data sources or columns change
Adjusted layer header padding for consistent spacing

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
VisualisationPanel.tsx	Updated grid column sizing from `minmax(0,1fr)` to `minmax(200px,1fr)`, added `min-w-0` to SelectTrigger components, and replaced fragment with div wrapper for proper grid spanning
SteppedColorEditor.tsx	Added useEffect hooks to track and reset stepped color steps on data source/column changes, introduced strict validation logic for step ranges, and refactored local step initialization
LayerHeader.tsx	Changed padding from `pt-3` to `py-3` for balanced vertical spacing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T15:26:40Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+      areaStats?.primary &&
+      minValue !== undefined &&
+      maxValue !== undefined &&
+      maxValue >= minValue &&


The condition maxValue > minValue is redundant because it's already checked by maxValue >= minValue on line 188. Additionally, dividing by zero isn't possible when these values are equal, so line 189 should be the only check needed. Consider removing line 188 or using only maxValue > minValue to avoid redundant conditions.

Suggested change

maxValue >= minValue &&

Copilot · 2026-01-20T15:26:40Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+    colorScheme,
+    isReversed,
+    viewConfig.customColor,


Including colorScheme, isReversed, and viewConfig.customColor in the dependency array will cause this effect to run whenever the user changes the color scheme. However, this effect is intended to reset steps only when the column or data source changes (as indicated by the comment on line 171). This could lead to unexpected behavior where changing the color scheme resets the user's custom step ranges. Consider removing these color-related dependencies if the intent is to only reset on column/data source changes.

Suggested change

colorScheme,

isReversed,

viewConfig.customColor,

Copilot · 2026-01-20T15:26:41Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+  // Simplified: if steps don't match current data range, always use defaults
  const stepRanges = useMemo(() => {
-    if (minValue === 0 && maxValue === 0) {
+    if (!minValue && !maxValue) {


This condition change introduces a bug. The original condition checked if both values equal 0, but the new condition using !minValue && !maxValue will also return true when minValue is 0 and maxValue is 0, which is a valid data range (a constant value of 0). This will incorrectly return an empty array for valid data. The condition should explicitly check for undefined/null or use the original equality check.

Copilot · 2026-01-20T15:26:41Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+
+    // Default: 3 steps evenly distributed (fall through if no steps or invalid)
+
    // Default: 3 steps evenly distributed


Duplicate comment. There are two identical comments "Default: 3 steps evenly distributed" on consecutive lines. Remove one of these duplicate comments.

Suggested change

// Default: 3 steps evenly distributed

Copilot · 2026-01-20T15:26:41Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+  // Clear invalid steps and set defaults when data range changes
  useEffect(() => {
-    if (!viewConfig.steppedColorSteps?.length && steps) {
+    if (!steps || steps.length === 0 || (!minValue && !maxValue)) {


This condition has the same bug as line 238. When minValue and maxValue are both 0 (a valid constant value), !minValue && !maxValue evaluates to true, causing the function to return early incorrectly. This should check for undefined/null values instead.

Suggested change

if (!steps || steps.length === 0 || (!minValue && !maxValue)) {

if (!steps || steps.length === 0 || minValue == null || maxValue == null) {

Copilot · 2026-01-20T15:26:42Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+      const firstStep = configSteps[0];
+      const lastStep = configSteps[configSteps.length - 1];
+
+      // Strict check: ALL steps must be within [minValue, maxValue]
+      const allStepsInRange = configSteps.every(
+        (step) =>
+          step.start >= minValue &&
+          step.end <= maxValue &&
+          step.start <= step.end,
+      );
+
+      const boundariesMatch =
+        firstStep &&
+        lastStep &&
+        Math.abs(firstStep.start - minValue) <= tolerance &&
+        Math.abs(lastStep.end - maxValue) <= tolerance;
+


This validation logic is duplicated from the stepRanges useMemo (lines 245-263). The exact same validation (checking allStepsInRange and boundariesMatch with the same tolerance and logic) appears in both places. Consider extracting this into a separate function to avoid duplication and improve maintainability.

Suggested change

const firstStep = configSteps[0];

const lastStep = configSteps[configSteps.length - 1];

// Strict check: ALL steps must be within [minValue, maxValue]

const allStepsInRange = configSteps.every(

(step) =>

step.start >= minValue &&

step.end <= maxValue &&

step.start <= step.end,

);

const boundariesMatch =

firstStep &&

lastStep &&

Math.abs(firstStep.start - minValue) <= tolerance &&

Math.abs(lastStep.end - maxValue) <= tolerance;

function validateSteppedColorConfig(

stepsToValidate: SteppedColorStep[],

min: number,

max: number,

tol: number,

): { allStepsInRange: boolean; boundariesMatch: boolean } {

const first = stepsToValidate[0];

const last = stepsToValidate[stepsToValidate.length - 1];

// Strict check: ALL steps must be within [min, max]

const allStepsInRange = stepsToValidate.every(

(step) =>

step.start >= min &&

step.end <= max &&

step.start <= step.end,

);

const boundariesMatch =

!!first &&

!!last &&

Math.abs(first.start - min) <= tol &&

Math.abs(last.end - max) <= tol;

return { allStepsInRange, boundariesMatch };

}

const { allStepsInRange, boundariesMatch } = validateSteppedColorConfig(

configSteps,

minValue,

maxValue,

tolerance,

);

Copilot · 2026-01-20T15:26:42Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+      const stepSize = (maxValue - minValue) / 3;
+      const defaultStepRanges = [
+        { start: minValue, end: minValue + stepSize },
+        { start: minValue + stepSize, end: minValue + stepSize * 2 },
+        { start: minValue + stepSize * 2, end: maxValue },
+      ];
+
+      // Calculate colors for default steps
+      const interpolator = getInterpolator(colorScheme, viewConfig.customColor);
+      const defaultSteps = defaultStepRanges.map((rangeItem, index) => {
+        const numSteps = defaultStepRanges.length;
+        const gradientPosition = numSteps > 1 ? index / (numSteps - 1) : 0;
+        const t = isReversed ? 1 - gradientPosition : gradientPosition;
+        const clampedT = Math.max(0, Math.min(1, t));
+        const color = interpolator(clampedT);
+
+        return {
+          start: rangeItem.start,
+          end: rangeItem.end,
+          color: color || "#cccccc",
+        };
+      });


This default step calculation logic is duplicated in multiple places in this file (lines 278-282 for ranges, lines 287-304 for steps with colors). The same calculation pattern appears three times. Consider extracting this into a shared helper function to reduce duplication and improve maintainability.

Copilot

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/app/map/[id]/components/Choropleth/utils.ts

Copilot · 2026-01-21T12:05:35Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+    defaultSteps,
+    savedSteppedColorSteps?.length,
+    updateViewConfig,
+    viewConfig.steppedColorStepsByKey,


The useEffect has defaultSteps in its dependency array, but defaultSteps is a memoized object that will create a new reference whenever hasValidRange, maxValue, or minValue changes. This could cause the effect to run more frequently than intended. Additionally, the effect calls updateViewConfig which is likely stable, but including the entire viewConfig.steppedColorStepsByKey object in dependencies could cause unnecessary re-renders. Consider using a more specific dependency or restructuring to avoid infinite loops.

Suggested change

defaultSteps,

savedSteppedColorSteps?.length,

updateViewConfig,

viewConfig.steppedColorStepsByKey,

hasValidRange,

minValue,

maxValue,

savedSteppedColorSteps?.length,

updateViewConfig,

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

Copilot · 2026-01-21T12:05:35Z

src/app/map/[id]/components/controls/VisualisationPanel/SteppedColorEditor.tsx

+  const [localSteps, setLocalSteps] = useState<SteppedColorStep[]>(
+    savedSteppedColorSteps || [],
+  );



When the dialog is opened, localSteps is not synchronized with the current savedSteppedColorSteps. The state is only initialized once on mount (line 198-200). If savedSteppedColorSteps changes after the component mounts (e.g., from another source or due to data loading), the dialog will display stale data. Consider adding a useEffect that updates localSteps when savedSteppedColorSteps changes or when the dialog opens.

Suggested change

// Keep localSteps in sync with saved steps or defaults when the dialog opens

useEffect(() => {

if (!isOpen) {

return;

}

if (savedSteppedColorSteps && savedSteppedColorSteps.length > 0) {

setLocalSteps(savedSteppedColorSteps);

} else if (hasValidRange) {

setLocalSteps(defaultSteps);

}

}, [isOpen, savedSteppedColorSteps, defaultSteps, hasValidRange]);

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…dColorEditor.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Arbyhisenaj added 3 commits January 20, 2026 11:00

fixing stepped colour issues

b58fb4c

undoing styling to layer header

c83a143

lint fix

0ff6359

joaquimds requested a review from Copilot January 20, 2026 15:22

Copilot started reviewing on behalf of joaquimds January 20, 2026 15:22 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Arbyhisenaj and others added 5 commits January 20, 2026 15:31

SteppedColourEditor Agent Refactor

d48b01a

removed uncessary datakey

845fd5d

Merge branch 'main' into jan26-QA-Fixes

768bb2a

Merge branch 'main' into jan26-QA-Fixes

baed269

refactor: stepped color selector

29c5c69

joaquimds requested a review from Copilot January 21, 2026 12:01

Copilot started reviewing on behalf of joaquimds January 21, 2026 12:02 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

joaquimds and others added 3 commits January 21, 2026 13:07

fix: npm dependencies issue

fb38841

Update src/app/map/[id]/components/Choropleth/utils.ts

4f5c18d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/app/map/[id]/components/controls/VisualisationPanel/Steppe…

0dfd60e

…dColorEditor.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

joaquimds merged commit 9d46b56 into main Jan 21, 2026
1 check passed

joaquimds deleted the jan26-QA-Fixes branch January 21, 2026 12:12


		// Default: 3 steps evenly distributed (fall through if no steps or invalid)

		// Default: 3 steps evenly distributed

	if (!steps \|\| steps.length === 0 \|\| (!minValue && !maxValue)) {
	if (!steps \|\| steps.length === 0 \|\| minValue == null \|\| maxValue == null) {

-      const firstStep = configSteps[0];
-      const lastStep = configSteps[configSteps.length - 1];
-      // Strict check: ALL steps must be within [minValue, maxValue]
-      const allStepsInRange = configSteps.every(
-        (step) =>
-          step.start >= minValue &&
-          step.end <= maxValue &&
-          step.start <= step.end,
-      );
-      const boundariesMatch =
-        firstStep &&
-        lastStep &&
-        Math.abs(firstStep.start - minValue) <= tolerance &&
-        Math.abs(lastStep.end - maxValue) <= tolerance;
+      function validateSteppedColorConfig(
+        stepsToValidate: SteppedColorStep[],
+        min: number,
+        max: number,
+        tol: number,
+      ): { allStepsInRange: boolean; boundariesMatch: boolean } {
+        const first = stepsToValidate[0];
+        const last = stepsToValidate[stepsToValidate.length - 1];
+        // Strict check: ALL steps must be within [min, max]
+        const allStepsInRange = stepsToValidate.every(
+          (step) =>
+            step.start >= min &&
+            step.end <= max &&
+            step.start <= step.end,
+        );
+        const boundariesMatch =
+          !!first &&
+          !!last &&
+          Math.abs(first.start - min) <= tol &&
+          Math.abs(last.end - max) <= tol;
+        return { allStepsInRange, boundariesMatch };
+      }
+      const { allStepsInRange, boundariesMatch } = validateSteppedColorConfig(
+        configSteps,
+        minValue,
+        maxValue,
+        tolerance,
+      );

+  // Keep localSteps in sync with saved steps or defaults when the dialog opens
+  useEffect(() => {
+    if (!isOpen) {
+      return;
+    }
+    if (savedSteppedColorSteps && savedSteppedColorSteps.length > 0) {
+      setLocalSteps(savedSteppedColorSteps);
+    } else if (hasValidRange) {
+      setLocalSteps(defaultSteps);
+    }
+  }, [isOpen, savedSteppedColorSteps, defaultSteps, hasValidRange]);

Uh oh!

Jan26 qa fixes #277

Jan26 qa fixes #277

Uh oh!

Conversation

Arbyhisenaj commented Jan 20, 2026

Description

Motivation and Context

How Can It Be Tested?

How Will This Be Deployed?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants