Improve color legend tick marks in `ColorLegendsContainer` #1169

HansKallekleiv · 2025-10-07T11:04:13Z

Algorithm to round tickmarks with proper positioning
Keep min/max on scale
Move code to utilities and add test

Copilot

Pull Request Overview

This PR improves the color legend tick marks in ColorLegendsContainer by implementing a new algorithm for generating nicely positioned tick marks with proper rounding. The changes ensure min/max values are preserved on the scale while using cleaner tick positioning.

Implements a new generateNiceAxisTicks utility function for calculating optimal tick positions
Replaces manual tick spacing calculations with smart algorithm that generates round numbers
Adds comprehensive test coverage for the new axis utilities

Reviewed Changes

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

File	Description
frontend/tests/unit/axisUtils.test.ts	Comprehensive test suite for the new axis tick generation functionality
frontend/src/modules/_shared/utils/numberFormatting.ts	Adds utility function to format numbers without trailing zeros
frontend/src/modules/_shared/utils/axisUtils.ts	New utility module with tick generation algorithm and spacing constraints
frontend/src/modules/_shared/components/ColorLegendsContainer/colorLegendsContainer.tsx	Refactored to use new tick generation utilities instead of manual calculations

Comments suppressed due to low confidence (1)

frontend/src/modules/_shared/utils/axisUtils.ts:1

[nitpick] The threshold 1e-10 should be extracted as a named constant like FLOATING_POINT_EPSILON to make the intent clearer and allow for easier adjustment.

export interface AxisTickOptions {

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-07T11:04:49Z

frontend/src/modules/_shared/utils/numberFormatting.ts

+    // Remove trailing zeros after decimal point, and remove decimal point if no digits follow
+    // This handles cases like: "1.000" -> "1", "1.200" -> "1.2", "1.20K" -> "1.2K"
+    return formatted.replace(/(\.\d*?)0+(\D|$)/, "$1$2").replace(/\.$/, "");


[nitpick] The regex pattern (\.\d*?)0+(\D|$) could be clearer. Consider breaking this into separate steps or adding a comment explaining the pattern matches trailing zeros followed by non-digit characters or end of string.

Suggested change

// Remove trailing zeros after decimal point, and remove decimal point if no digits follow

// This handles cases like: "1.000" -> "1", "1.200" -> "1.2", "1.20K" -> "1.2K"

return formatted.replace(/(\.\d*?)0+(\D|$)/, "$1$2").replace(/\.$/, "");

// Remove trailing zeros from the decimal part (e.g., "1.200" -> "1.2", "1.20K" -> "1.2K")

const noTrailingZeros = formatted.replace(/(\.\d*?[1-9])0+(\D|$)/, "$1$2");

// Remove dangling decimal point if no digits follow (e.g., "1." -> "1")

return noTrailingZeros.replace(/\.([^\d]|$)/, "$1");

Copilot · 2025-10-07T11:04:50Z

frontend/src/modules/_shared/utils/axisUtils.ts

+        if (effectiveMaxTicks < 2 && options.availableSpace >= options.minTickSpacing) {
+            effectiveMaxTicks = 2;
+        }


[nitpick] The magic number 2 should be extracted as a named constant like MIN_TICKS_WITH_BOUNDARIES to clarify that this ensures at least min and max ticks are included.

Anders2303 · 2025-10-09T08:48:52Z

frontend/src/modules/_shared/utils/axisUtils.ts

+ * // Returns fewer ticks to prevent overlap
+ */
+
+export function generateNiceAxisTicks(min: number, max: number, maxTicks: number, options?: AxisTickOptions): number[] {


I feel like "nice" is a bit too vague, consider something like generateRoundedAxisTicks

Anders2303 · 2025-10-09T08:51:17Z

frontend/src/modules/_shared/utils/axisUtils.ts

+    const actualMin = Math.min(min, max);
+    const actualMax = Math.max(min, max);
+    const range = actualMax - actualMin;
+    const MIN_TICK_WITH_BOUNDARIES = 2; // Minimum ticks to ensure min/max is included


You should write constants like this at the top level of the file

Anders2303 · 2025-10-09T09:16:47Z

frontend/tests/unit/axisUtils.test.ts

+
+        test("should handle asymmetric ranges around zero", () => {
+            const result = generateNiceAxisTicks(-2, 8, 5);
+            expect(result).toEqual([-2, 0, 5, 8]);


Isn't this uneven? The steps between ticks here is 2, 5, 3, but I'd expect something like3,4,3

Anders2303 · 2025-10-09T09:19:50Z

frontend/tests/unit/axisUtils.test.ts

+    describe("edge cases", () => {
+        test("should handle maxTicks of 1", () => {
+            const result = generateNiceAxisTicks(0, 100, 1);
+            expect(result.length).toBeGreaterThanOrEqual(1);


The resulting list shouldn't be allowed to be longer than the maxTicks value, no? Shouldnt this test be expect(result.length).toBeLessThanOrEqual(1)

Anders2303 · 2025-10-09T09:20:09Z

frontend/tests/unit/axisUtils.test.ts

+
+        test("should handle maxTicks of 2", () => {
+            const result = generateNiceAxisTicks(0, 100, 2);
+            expect(result.length).toBeGreaterThanOrEqual(2);


Anders2303 · 2025-10-09T10:18:44Z

frontend/src/modules/_shared/utils/axisUtils.ts

+ * // Returns fewer ticks to prevent overlap
+ */
+
+export function generateNiceAxisTicks(min: number, max: number, maxTicks: number, options?: AxisTickOptions): number[] {


As discussed, might be slightly too aggressive on the computation; these should be able to fit 3/5 ticks

For instance, with the range (-178.726, 762.265), if we consider tens to be a round number, we could have

I sketched up a very quick test in python, I think something like this should work? You'd still need to figure out how to best decide what multiple to round to

real_start = -178.726 real_end = 762.265 ticks = 5 rounding_digits = -1 # Round to the limits outside the round_start = round(real_start, rounding_digits) # Round to tens round_end = round(real_end, rounding_digits) + 10**(-rounding_digits) # Round to tens distance = round_end - round_start stepSize = round(distance / (ticks - 1), rounding_digits) # stepSize should be the same rounding rounded_ticks = [(round_start + t * stepSize) for t in range(1, ticks - 1)] rounded_ticks.insert(0, real_start) rounded_ticks.append(real_end)

Rounded limits: -180.0 , 770.0 Step size: 240.0 Rounded ticks: [-178.726, 60.0, 300.0, 540.0, 762.265]

anders-kiaer · 2025-10-29T07:51:46Z

Putting this on hold for now, until after persistance. @HansKallekleiv and @rubenthoms discussed the need of also supporting log scales.

HansKallekleiv added 2 commits October 7, 2025 12:14

Show rounded ticks in Colorscale

4df3e86

Handle boundaries. Add tests

09569f2

HansKallekleiv requested a review from Copilot October 7, 2025 11:04

Copilot AI reviewed Oct 7, 2025

View reviewed changes

lint

77e9b2c

HansKallekleiv requested review from Anders2303 and rubenthoms October 7, 2025 11:17

Anders2303 requested changes Oct 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve color legend tick marks in `ColorLegendsContainer` #1169

Improve color legend tick marks in `ColorLegendsContainer` #1169

HansKallekleiv commented Oct 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 7, 2025

Uh oh!

Copilot AI Oct 7, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

Anders2303 Oct 9, 2025

Uh oh!

anders-kiaer commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    // Remove trailing zeros after decimal point, and remove decimal point if no digits follow
-    // This handles cases like: "1.000" -> "1", "1.200" -> "1.2", "1.20K" -> "1.2K"
-    return formatted.replace(/(\.\d*?)0+(\D|$)/, "$1$2").replace(/\.$/, "");
+    // Remove trailing zeros from the decimal part (e.g., "1.200" -> "1.2", "1.20K" -> "1.2K")
+    const noTrailingZeros = formatted.replace(/(\.\d*?[1-9])0+(\D|$)/, "$1$2");
+    // Remove dangling decimal point if no digits follow (e.g., "1." -> "1")
+    return noTrailingZeros.replace(/\.([^\d]|$)/, "$1");

Improve color legend tick marks in ColorLegendsContainer #1169

Are you sure you want to change the base?

Improve color legend tick marks in ColorLegendsContainer #1169

Conversation

HansKallekleiv commented Oct 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Anders2303 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

anders-kiaer commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Improve color legend tick marks in `ColorLegendsContainer` #1169

Improve color legend tick marks in `ColorLegendsContainer` #1169