Skip to content

fix: scale colorbar tick font size#688

Open
dhanush-kannan-dk wants to merge 3 commits into
masterfrom
fix/colorbar-tick-density-679
Open

fix: scale colorbar tick font size#688
dhanush-kannan-dk wants to merge 3 commits into
masterfrom
fix/colorbar-tick-density-679

Conversation

@dhanush-kannan-dk

Copy link
Copy Markdown
Contributor

Fixes #679

Summary: The colorbar below the map had a fixed font size and number format
regardless of container width, causing tick labels to overlap on narrow
viewports. The zoom control buttons were also oversized on small screens.

Changes

  • User experience
    • Colorbar tick labels now scale font size continuously based on container
      width (0.5em at ≤450px → 1em at ≥900px), preventing overlap when the
      map is in a narrow layout
    • Colorbar number labels switch to short notation (K, M, B) below 700px
      wide, replacing long labels like "190 thousand" with "190K"
    • Zoom control icons and button padding shrink on narrow screens
      (16px/0.25em at <500px → 20px/0.35em at <700px → 24px/0.5em at ≥700px)
  • Logical changes
    • ResizeObserver added to ColorBar to track its own pixel width
    • ResizeObserver added to EntityMap outer container; width passed
      down to ZoomControls as a prop
  • Refactors
    • Fixed a pre-existing React hooks violation in ColorBar where useMemo
      was called after an early return null
    • Fixed a locale-sensitive test in InteractiveEntityTable.test.tsx that
      hardcoded '1,234,567' — replaced with toLocaleString() so it passes
      on machines using non-Western number formatting (e.g. Indian lakh system)

Out of scope/Future work: Could further reduce the number of ticks shown at
very narrow widths if font scaling alone is not sufficient.

Test Plan and Screenshots

How to test the changes in this PR: Run npm run dev, navigate to a map
view, enable Color By (e.g. Population), then resize the browser window
narrower to observe font size and number format adapting. Use DevTools
device toolbar set to iPhone SE (375px) to verify zoom button sizing.

Testing

  • npm run lint
  • npm run build
  • npm run test
    • Test updated for one failed test case
  • npm run dev -- tried out the website directly
    • Include screenshots as noted below
image

@dhanush-kannan-dk dhanush-kannan-dk requested a review from a team as a code owner June 25, 2026 11:17
@dhanush-kannan-dk dhanush-kannan-dk self-assigned this Jun 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Cloudflare Pages preview

@conradarcturus conradarcturus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great :) Changes look good and it shows that you have an eye for detail. I have some stylistic notes but you'll welcome to ignore them if you prefer your choices.

)
: undefined;

if (minValue === undefined || maxValue === undefined) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the react problem by moving this after the hooks.

Comment on lines +40 to +41
const containerRef = useRef<HTMLDivElement>(null);
const [containerWidth, setContainerWidth] = useState(800);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Call this colorBarRef and colorBarWidth -- container is too generic.

const formatter = new Intl.NumberFormat(undefined, {
notation: 'compact',
compactDisplay: 'long', // or 'long' for “thousand”, “million”
compactDisplay: widthPx < 700 ? 'short' : 'long',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice call

justifyContent: 'center',
};
return (
<div style={containerStyle}>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of these padding & icon breaks, since padding and icon uses relative units (em) -- why not just change the container's font size?

Your approach works but it requires passing in a lot of styles when CSS already is configured to use relative sizing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Map Colorbar: Calibrate the # of ticks to be shown based on the pixel width

2 participants