Skip to content

Commit

Permalink
Fixup: Address reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
yvonnesjy committed Jun 20, 2024
1 parent 3951a6d commit ff6c846
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 31 deletions.
46 changes: 28 additions & 18 deletions src/js/common/Utilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,43 +106,53 @@ define([], () => {
return names;
},

// Format the number into a string with better readability.
formatNumber(value, min, max) {
if (!value && value !== 0) {
/**
* Format the number into a string with better readability, based on the manitude of a
* range this number falls in.
* @param {number} value The number value to be formatted.
* @param {number} range The range of numerics this value can fall in.
* @returns {string} A formatted number based on the magnitude of `range`.
*/
formatNumber(value, range) {
if (typeof value !== "number") {
return "";
}
if (typeof range !== "number") {
return value.toString();
}

const roundingConstant = Utilities.getRoundingConstant(max - min);
if (roundingConstant) {
return (
Math.round(value * roundingConstant) / roundingConstant
).toString();
const numDecimalPlaces = Utilities.getNumDecimalPlaces(range);
if (numDecimalPlaces !== null) {
return value.toFixed(numDecimalPlaces);
}
return value.toExponential(2).toString();
},

// Calculate the rounding precision we should use based on the
// range of the data.
getRoundingConstant(range) {
/**
* Calculate the number of decimal places we should use based on the range of the data.
* @param {number} range The range of data values.
* @returns {number} The number of decimal places we should use.
*/
getNumDecimalPlaces(range) {
if (range < 0.0001 || range > 100000) {
return null; // Will use scientific notation
}
if (range < 0.001) {
return 100000; // Allow 5 decimal places
return 5; // Allow 5 decimal places
}
if (range < 0.01) {
return 10000; // Allow 4 decimal places
return 4; // Allow 4 decimal places
}
if (range < 0.1) {
return 1000; // Allow 3 decimal places
return 3; // Allow 3 decimal places
}
if (range < 1) {
return 100; // Allow 2 decimal places
return 2; // Allow 2 decimal places
}
if (range > 100) {
return 1; // No decimal places
if (range <= 100) {
return 1; // Allow 1 decimal places
}
return 10; // Allow 1 decimal place by default
return 0; // No decimal places
},
};

Expand Down
8 changes: 3 additions & 5 deletions src/js/views/maps/LegendView.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ define([
const min = data[0].value;
const max = data[data.length - 1].value;
const range = max - min;
const roundingConstant = Utilities.getRoundingConstant(range);
const numDecimalPlaces = Utilities.getNumDecimalPlaces(range);

// SVG element
const svg = this.createSVG({
Expand Down Expand Up @@ -429,10 +429,8 @@ define([
// Show tooltip with the value
if (value || value === 0) {
// Round or show in scientific notation
if (roundingConstant) {
value = (
Math.round(value * roundingConstant) / roundingConstant
).toString();
if (numDecimalPlaces !== null) {
value = value.toFixed(numDecimalPlaces);
} else {
value = value.toExponential(2).toString();
}
Expand Down
7 changes: 4 additions & 3 deletions src/js/views/maps/legend/ContinuousSwatchView.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ define([
const min = this.collection.first().get("value");
const max = this.collection.last().get("value");
const mid = (min + max) / 2;
const range = max - min;
this.$el.html(
this.template({
classNames: CLASS_NAMES,
min: Utilities.formatNumber(min, min, max),
mid: Utilities.formatNumber(mid, min, max),
max: Utilities.formatNumber(max, min, max),
min: Utilities.formatNumber(min, range),
mid: Utilities.formatNumber(mid, range),
max: Utilities.formatNumber(max, range),
}),
);

Expand Down
18 changes: 13 additions & 5 deletions test/js/specs/unit/common/Utilities.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,25 @@ define(["../../../../../../src/js/common/Utilities"], function (EntityUtils) {

describe("formatNumber", () => {
it("rounds number if the range is between 0.0001 and 100000", () => {
expect(EntityUtils.formatNumber(0.000099999, 0, 0.0001)).to.equal(
"0.0001",
expect(EntityUtils.formatNumber(0.000099999, 0.0001)).to.equal(
"0.00010",
);
expect(EntityUtils.formatNumber(1.9, 0, 100000)).to.equal("2");
expect(EntityUtils.formatNumber(1.9, 100000)).to.equal("2");
});

it("uses scientific notation if the range is outside of 0.0001 and 100000", () => {
expect(EntityUtils.formatNumber(0.000099999, 0, 0.000099999)).to.equal(
expect(EntityUtils.formatNumber(0.000099999, 0.000099999)).to.equal(
"1.00e-4",
);
expect(EntityUtils.formatNumber(1.9, 0, 100001)).to.equal("1.90e+0");
expect(EntityUtils.formatNumber(1.9, 100001)).to.equal("1.90e+0");
});

it("returns empty string if input value isn't a number", () => {
expect(EntityUtils.formatNumber("1.0", 0.000099999)).to.equal("");
});

it("returns value as is if range isn't a number", () => {
expect(EntityUtils.formatNumber(1.9, "invalid range")).to.equal("1.9");
});
});
});
Expand Down

0 comments on commit ff6c846

Please sign in to comment.