search

mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.2k stars 2.22k forks source link

Inconsistent unit rendering in ScaleControl #12620

Closed AndreasHogstrom closed 1 year ago

AndreasHogstrom commented 1 year ago

ScaleControl with unit nautical renders with space between value and unit while other units (That are handled by Intl.NumberFormat) renders without space (unitDisplay: "narrow").

Screenshot 2023-03-20 at 15 43 06

mapbox-gl-js version: 2.13.0 and current main branch

browser: Any

Steps to Trigger Behavior

  1. Add ScaleControl with unit "nautical"
  2. Add ScaleControl with any other unit
  3. Inspect the difference

Link to Demonstration

https://jsfiddle.net/sy4gpxrL/

Expected Behavior

Consistent formatting between unit types.

Actual Behavior

Inconsistent formatting.

stepankuzmin commented 1 year ago

Hi @AndreasHogstrom,

Thanks for creating an issue. It seems that there is a leftover in the ScaleControl._setScale function with nautical unit.

https://github.com/mapbox/mapbox-gl-js/blob/2f65b40a87cd8d5113f9ac62ab4f0ff1a49d0bd1/src/ui/control/scale_control.js#L108

Do you want and take a stab at it and remove the non-breaking space?

kathirgounder commented 1 year ago

Hi @stepankuzmin ! Mind if I draft up a PR to remove the non-breaking space, wanted to start contributing to the repo, thought it would be nice to get this simple one under the belt, thank you!

stepankuzmin commented 1 year ago

Sure, go ahead, @kathirgounder 👍

kathirgounder commented 1 year ago

Hi @stepankuzmin ! I drafted a small PR up, was wondering if you had any clue on that test that is failing (if its something flaky or not because it seems unrelated), and on any other steps I should complete for this PR (think I need to still do the changelog since its a visual change and also tag the design team per the launch checklist). Really appreciate it!

stepankuzmin commented 1 year ago

Done in https://github.com/mapbox/mapbox-gl-js/pull/12644