diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bdc482..89476dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Enable setting type weights to 0 in the UI (#1005) - Add support for 🚀 and 👀 reaction types (#1068) - Create one page per project, rather than having a selector (#988) diff --git a/src/explorer/weights/WeightSlider.js b/src/explorer/weights/WeightSlider.js index 289cb18..80921f8 100644 --- a/src/explorer/weights/WeightSlider.js +++ b/src/explorer/weights/WeightSlider.js @@ -2,11 +2,55 @@ import React from "react"; +/** + * The Weight that the user supplies via a WeightSlider. + * Weights are finite and non-negative. The vast majority of weights cannot + * be selected via the WeightSlider, which is only capable of choosing a few + * specific weights. + */ +export type Weight = number; + +/** + * SliderPosition is an integer in the range `[MIN_SLIDER, MAX_SLIDER]` which + * describes the current position of the WeightSlider's slider. + */ +export type SliderPosition = number; + +export const MIN_SLIDER: SliderPosition = -5; +export const MAX_SLIDER: SliderPosition = 5; + export type Props = {| - +weight: number, + // The current Weight the WeightSlider is set to + // Note: It is acceptable to pass the WeightSlider a weight which the slider + // is incapable of picking (e.g. 1/3). + +weight: Weight, + // The name associated with the slider; displayed adjacent to the slider. +name: React$Node, - +onChange: (number) => void, + // Callback invoked with the new weight after the user adjusts the slider. + +onChange: (Weight) => void, |}; + +/** + * The WeightSlider is a [controlled component] which lets the user choose a + * numeric weight by dragging a slider. + * + * The weight varies exponentially in the slider position, so that moving the + * slider by one notch changes the weight by a power of two. The exception is + * when the user drags the slider to its minimum value, in which case the + * weight is set to 0. + * + * In addition to rendering a slider (instantiated as a [range input]), it also renders + * a description of the slider, and the weight associated with the current slider position. + * + * Note that it is possible to configure the WeightSlider with a weight that + * that is impossible to select via the slider; for example, 1/3. In such cases, + * the WeightSlider's slider will be set to the closest possible position, and after + * the user adjusts the weight via the slider, it will always correspond exactly to + * the slider position. + * + * [controlled component]: https://reactjs.org/docs/forms.html#controlled-components + * [range input]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range + */ export class WeightSlider extends React.Component { render() { return ( @@ -14,13 +58,13 @@ export class WeightSlider extends React.Component { {this.props.name} { - const logValue = e.target.valueAsNumber; - this.props.onChange(2 ** logValue); + const weight: Weight = sliderToWeight(e.target.valueAsNumber); + this.props.onChange(weight); }} />{" "} { } } +/** + * Converts a Weight into a SliderPosition. + * + * If the Weight is an illegal value (negative, NaN, or infinite), an error + * will be thrown. + * + * It is possible that the Weight will be a legal value, but will not + * correspond to any SliderPosition (as SliderPositions are always integers, in + * the range `[MIN_SLIDER, MAX_SLIDER]`). In such cases, it may seem principled + * to throw a conversion error. However, initial weights are provided by + * plugins, and plugin authors might reasonably choose weights like 1/3 that do + * not correspond to a slider value. We choose to make the WeightSlider robust + * to such decisions by rounding to the closest appropriate slider position. In + * such cases, the WeightSlider's weight will be inconsistent with the slider + * position, and moving the Slider away from and back to its initial position + * will actually change the weight. This is unfortunate, but is preferable to + * having the application error. + */ +export function weightToSlider(weight: Weight): SliderPosition { + if (!isFinite(weight) || weight < 0) { + throw new Error(`illegal weight: ${weight}`); + } + const tentativePosition = Math.log2(weight); + if (tentativePosition < MIN_SLIDER) { + return MIN_SLIDER; + } + if (tentativePosition > MAX_SLIDER) { + return MAX_SLIDER; + } + return Math.round(tentativePosition); +} + +/** + * Converts a SliderPosition into a Weight. + * + * SliderPosition must be an integer in the range `[MIN_SLIDER, MAX_SLIDER]`. + * If it is not, then an error will be thrown. + * + * Over the domain of legal slider positions `p`, `weightToSlider(sliderToWeight(p))` + * is the identity function. + */ +export function sliderToWeight(sliderPosition: SliderPosition): Weight { + if (sliderPosition < MIN_SLIDER || sliderPosition > MAX_SLIDER) { + throw new Error(`Slider position out of range: ${sliderPosition}`); + } + if (isNaN(sliderPosition)) { + throw new Error("illegal value: NaN"); + } + if (Math.round(sliderPosition) !== sliderPosition) { + throw new Error(`slider position not integer: ${sliderPosition}`); + } + return sliderPosition === MIN_SLIDER ? 0 : 2 ** sliderPosition; +} + export function formatWeight(n: number) { - if (n <= 0 || !isFinite(n)) { + if (n < 0 || !isFinite(n)) { throw new Error(`Invalid weight: ${n}`); } - if (n >= 1) { + if (n >= 1 || n === 0) { return n.toFixed(0) + "×"; } else { return `1/${(1 / n).toFixed(0)}×`; diff --git a/src/explorer/weights/WeightSlider.test.js b/src/explorer/weights/WeightSlider.test.js index a44e36b..68c7b32 100644 --- a/src/explorer/weights/WeightSlider.test.js +++ b/src/explorer/weights/WeightSlider.test.js @@ -3,34 +3,50 @@ import React from "react"; import {shallow} from "enzyme"; -import {WeightSlider, formatWeight} from "./WeightSlider"; +import { + WeightSlider, + formatWeight, + MIN_SLIDER, + MAX_SLIDER, + sliderToWeight, + weightToSlider, + type Weight, +} from "./WeightSlider"; require("../../webutil/testUtil").configureEnzyme(); describe("explorer/weights/WeightSlider", () => { describe("WeightSlider", () => { - function example() { + function example(weight: Weight) { const onChange = jest.fn(); const element = shallow( - + ); return {element, onChange}; } - it("sets slider to the log of provided weight", () => { - const {element} = example(); - expect(element.find("input").props().value).toBe(Math.log2(3)); + // These are all valid weights, but not all of them correspond to a valid + // slider position. + const exampleWeights = [0, 2 ** -10, 0.25, 0.33, 1, 20]; + it("sets the slider as corresponds to the current weight", () => { + for (const w of exampleWeights) { + const expectedSlider = weightToSlider(w); + const {element} = example(w); + expect(element.find("input").props().value).toBe(expectedSlider); + } }); it("prints the provided weight", () => { - const {element} = example(); - expect( - element - .find("span") - .at(1) - .text() - ).toBe(formatWeight(3)); + for (const w of exampleWeights) { + const {element} = example(w); + expect( + element + .find("span") + .at(1) + .text() + ).toBe(formatWeight(w)); + } }); it("displays the provided name", () => { - const {element} = example(); + const {element} = example(0); expect( element .find("span") @@ -38,15 +54,83 @@ describe("explorer/weights/WeightSlider", () => { .text() ).toBe("foo"); }); - it("changes to the slider trigger the onChange with exponentiatied value", () => { - const {element, onChange} = example(); + it("changes to the slider trigger the onChange with the corresponding weight", () => { + const sliderVals = [MIN_SLIDER, 0, MAX_SLIDER]; + for (const sliderVal of sliderVals) { + const {element, onChange} = example(0); + const input = element.find("input"); + input.simulate("change", {target: {valueAsNumber: sliderVal}}); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith(sliderToWeight(sliderVal)); + } + }); + it("the weight and slider position may be inconsistent", () => { + // If the weight does not correspond to an integer slider value, then + // changing the slider to its current position can change the weight. + // See the docstring on `weightToSlider` for justification. + const imperfectWeight = 1 / 3; + const {element, onChange} = example(imperfectWeight); const input = element.find("input"); - input.simulate("change", {target: {valueAsNumber: 7}}); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith(2 ** 7); + const elementSliderValue = input.props().value; + input.simulate("change", {target: {valueAsNumber: elementSliderValue}}); + expect(onChange).not.toHaveBeenCalledWith(imperfectWeight); }); }); + describe("weight<-> slider conversions", () => { + it("slider->weight->slider is identity", () => { + const legalSliderPositions = [MIN_SLIDER, 0, MAX_SLIDER]; + for (const sliderPosition of legalSliderPositions) { + const weight = sliderToWeight(sliderPosition); + const position_ = weightToSlider(weight); + expect(sliderPosition).toEqual(position_); + } + }); + it("weightToSlider truncates when out of range", () => { + const tinyWeight = sliderToWeight(MIN_SLIDER) / 2; + expect(weightToSlider(tinyWeight)).toEqual(MIN_SLIDER); + + const giantWeight = sliderToWeight(MAX_SLIDER) * 2; + expect(weightToSlider(giantWeight)).toEqual(MAX_SLIDER); + }); + it("weightToSlider errors on invalid weights", () => { + const invalid = [NaN, Infinity, -Infinity, -1]; + for (const v of invalid) { + expect(() => weightToSlider(v)).toThrowError("illegal weight"); + } + }); + it("weightToSlider rounds to closest corresponding slider value", () => { + const nonIntegerSliders = [-0.3, 0.3, 0.9]; + for (const nonIntegerSlider of nonIntegerSliders) { + const w = 2 ** nonIntegerSlider; + expect(weightToSlider(w)).toEqual(Math.round(nonIntegerSlider)); + } + }); + it("sliderToWeight errors on slider position out of range", () => { + const illegalValues = [ + -Infinity, + MIN_SLIDER - 1, + MAX_SLIDER + 1, + Infinity, + ]; + for (const illegalValue of illegalValues) { + expect(() => sliderToWeight(illegalValue)).toThrowError( + "Slider position out of range" + ); + } + }); + it("sliderToWeight errors on non-integer values", () => { + const nonIntegers = [-0.3, 0.3, 0.9]; + for (const nonInteger of nonIntegers) { + expect(() => sliderToWeight(nonInteger)).toThrowError( + "slider position not integer" + ); + } + }); + it("sliderToWeight errors on NaN", () => { + expect(() => sliderToWeight(NaN)).toThrowError("illegal value: NaN"); + }); + }); describe("formatWeight", () => { it("shows numbers greater than 1 as a integer-rounded multiplier", () => { expect(formatWeight(5.3)).toBe("5×"); @@ -54,8 +138,11 @@ describe("explorer/weights/WeightSlider", () => { it("shows numbers less than 1 (but not 0) as integer-rounded fractions", () => { expect(formatWeight(0.249)).toBe("1/4×"); }); + it("shows numbers equal to 0 as 0x", () => { + expect(formatWeight(0)).toBe("0×"); + }); it("throws on bad values", () => { - const bads = [NaN, Infinity, -Infinity, -3, 0]; + const bads = [NaN, Infinity, -Infinity, -3]; for (const bad of bads) { expect(() => formatWeight(bad)).toThrowError("Invalid weight"); } diff --git a/src/plugins/github/declaration.js b/src/plugins/github/declaration.js index b5656fa..7e883a6 100644 --- a/src/plugins/github/declaration.js +++ b/src/plugins/github/declaration.js @@ -101,7 +101,7 @@ const referencesEdgeType = Object.freeze({ forwardName: "references", backwardName: "is referenced by", defaultForwardWeight: 1, - defaultBackwardWeight: 1 / 16, + defaultBackwardWeight: 0, prefix: E.Prefix.references, description: "TODO: Add a description for this EdgeType", }); @@ -110,8 +110,7 @@ const mentionsAuthorEdgeType = Object.freeze({ forwardName: "mentions author of", backwardName: "has author mentioned by", defaultForwardWeight: 1, - // TODO(#811): Probably change this to 0 - defaultBackwardWeight: 1 / 32, + defaultBackwardWeight: 0, prefix: E.Prefix.mentionsAuthor, description: "TODO: Add a description for this EdgeType", }); @@ -120,8 +119,7 @@ const reactsHeartEdgeType = Object.freeze({ forwardName: "reacted ❤️ to", backwardName: "got ❤️ from", defaultForwardWeight: 2, - // TODO(#811): Probably change this to 0 - defaultBackwardWeight: 1 / 32, + defaultBackwardWeight: 0, prefix: E.Prefix.reactsHeart, description: "TODO: Add a description for this EdgeType", }); @@ -130,8 +128,7 @@ const reactsThumbsUpEdgeType = Object.freeze({ forwardName: "reacted 👍 to", backwardName: "got 👍 from", defaultForwardWeight: 1, - // TODO(#811): Probably change this to 0 - defaultBackwardWeight: 1 / 32, + defaultBackwardWeight: 0, prefix: E.Prefix.reactsThumbsUp, description: "TODO: Add a description for this EdgeType", }); @@ -140,8 +137,7 @@ const reactsHoorayEdgeType = Object.freeze({ forwardName: "reacted 🎉 to", backwardName: "got 🎉 from", defaultForwardWeight: 4, - // TODO(#811): Probably change this to 0 - defaultBackwardWeight: 1 / 32, + defaultBackwardWeight: 0, prefix: E.Prefix.reactsHooray, description: "TODO: Add a description for this EdgeType", }); @@ -150,8 +146,7 @@ const reactsRocketEdgeType = Object.freeze({ forwardName: "reacted 🚀 to", backwardName: "got 🚀 from", defaultForwardWeight: 1, - // TODO(#811): Probably change this to 0 - defaultBackwardWeight: 1 / 32, + defaultBackwardWeight: 0, prefix: E.Prefix.reactsRocket, description: "TODO: Add a description for this EdgeType", });