Skip to content

Commit

Permalink
UI polish for cohort select menu (#254)
Browse files Browse the repository at this point in the history
  • Loading branch information
macfarlandian committed Nov 17, 2020
1 parent 1f64748 commit 35b171e
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 16 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
working-directory: public-dashboard-client
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1.1.0
- uses: actions/setup-node@v1
with:
node-version: "12.x"
- uses: c-hive/gha-yarn-cache@v1
Expand All @@ -27,7 +27,7 @@ jobs:
working-directory: public-dashboard-client
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1.1.0
- uses: actions/setup-node@v1
with:
node-version: "12.x"
- uses: c-hive/gha-yarn-cache@v1
Expand All @@ -48,7 +48,7 @@ jobs:
working-directory: spotlight-client
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1.1.0
- uses: actions/setup-node@v1
with:
node-version: "12.x"
- uses: c-hive/gha-yarn-cache@v1
Expand All @@ -62,7 +62,7 @@ jobs:
working-directory: spotlight-client
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1.1.0
- uses: actions/setup-node@v1
with:
node-version: "12.x"
- uses: c-hive/gha-yarn-cache@v1
Expand All @@ -83,7 +83,7 @@ jobs:
working-directory: spotlight-api
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1.1.0
- uses: actions/setup-node@v1
with:
node-version: "12.x"
- uses: c-hive/gha-yarn-cache@v1
Expand Down
1 change: 1 addition & 0 deletions public-dashboard-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"d3-force-limit": "^1.1.3",
"d3-format": "^1.4.4",
"d3-geo": "^1.12.1",
"d3-interpolate": "^2.0.1",
"d3-scale": "^3.2.1",
"date-fns": "^2.14.0",
"deepmerge": "^4.2.2",
Expand Down
32 changes: 29 additions & 3 deletions public-dashboard-client/src/controls/CohortSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
DropdownWrapper as DropdownWrapperBase,
HiddenSelect,
} from "./shared";
import { highlightFade } from "../utils";

const SELECT_ALL_ID = "ALL";

Expand Down Expand Up @@ -91,15 +92,33 @@ const OPTIONS_PROP_TYPE = PropTypes.arrayOf(
and([DropdownOptionType, PropTypes.shape({ color: PropTypes.string })])
);

function getOptionColor({ isSelected, highlightedIndex, index, opt }) {
// by default there is no color
let color;
if (isSelected) {
// a selected option has either the dataviz color or its faded variant
// depending on whether a menu hover state is active
// zero index is ignored because it's the "select all" option
if (highlightedIndex > 0 && highlightedIndex !== index) {
color = highlightFade(opt.color);
} else {
color = opt.color;
}
}
return color;
}

function CustomSelect({
buttonContents,
onHighlight,
options: optionsFromData,
selected,
setSelected,
}) {
const selectAllLabel =
selected.length === optionsFromData.length ? "Deselect all" : "Select all";
const visibleOptions = [
{ id: SELECT_ALL_ID, label: "Select all" },
{ id: SELECT_ALL_ID, label: selectAllLabel },
...optionsFromData,
];

Expand Down Expand Up @@ -191,15 +210,22 @@ function CustomSelect({
{...itemProps}
aria-selected={isSelected}
as="li"
backgroundColor={isSelected ? opt.color : undefined}
backgroundColor={getOptionColor({
highlightedIndex,
index,
isSelected,
opt,
})}
// color in this menu does not change because we just fade the others instead
highlightColor={opt.color}
highlightedSelector={
highlightedIndex === index ? `&#${itemProps.id}` : undefined
}
key={opt.id}
>
<MenuItemContents>
{opt.label}
<MenuItemCheckMark src={checkMarkPath} />
<MenuItemCheckMark alt="" src={checkMarkPath} />
</MenuItemContents>
</DropdownMenuItem>
);
Expand Down
29 changes: 28 additions & 1 deletion public-dashboard-client/src/controls/CohortSelect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import userEvent from "@testing-library/user-event";
import useBreakpoint from "@w11r/use-breakpoint";
import React from "react";
import { act, render, within } from "../testUtils";
import { highlightFade } from "../utils";
import CohortSelect from "./CohortSelect";

jest.mock("@w11r/use-breakpoint");
Expand Down Expand Up @@ -196,12 +197,32 @@ test("applies colors to selected items", () => {
).toHaveStyle(`background-color: ${opt.color}`);
});
const firstOption = getByRole("option", { name: testOptions[0].label });
act(() => userEvent.click(firstOption));

userEvent.click(firstOption);
// item remains highlighted while the cursor is pointed at it
userEvent.unhover(firstOption);

expect(firstOption).not.toHaveStyle(
`background-color: ${testOptions[0].color}`
);
});

test("fades colors on hover", () => {
const { getByRole } = openMenu();
const firstOption = getByRole("option", { name: testOptions[0].label });

userEvent.hover(firstOption);

// highlighted item should remain the same; others should fade
expect(firstOption).toHaveStyle(`background-color: ${testOptions[0].color}`);
testOptions.slice(1).forEach((opt) => {
const fadedColor = highlightFade(opt.color);
expect(getByRole("option", { name: opt.label })).toHaveStyle(
`background-color: ${fadedColor}`
);
});
});

test("passes highlighted option to callback", () => {
const { getByRole } = openMenu();
testOptions.forEach((opt) => {
Expand All @@ -216,6 +237,7 @@ test("supports select-all", () => {
const { getByRole } = openMenu();
const selectAll = getByRole("option", { name: /select all/i });

expect(selectAll).toHaveTextContent(/deselect all/i);
act(() => userEvent.click(selectAll));

// de-selects all
Expand All @@ -225,6 +247,7 @@ test("supports select-all", () => {
"false"
);
});
expect(selectAll).toHaveTextContent(/(?<!de)select all/i);

// click again to select all
act(() => userEvent.click(selectAll));
Expand All @@ -234,6 +257,7 @@ test("supports select-all", () => {
"true"
);
});
expect(selectAll).toHaveTextContent(/deselect all/i);

// now de-select some manually and try again
testOptions.slice(2, 6).forEach((opt) => {
Expand All @@ -244,6 +268,8 @@ test("supports select-all", () => {
);
});

expect(selectAll).toHaveTextContent(/(?<!de)select all/i);

act(() => userEvent.click(selectAll));
// everything is selected again
testOptions.forEach((opt) => {
Expand All @@ -252,4 +278,5 @@ test("supports select-all", () => {
"true"
);
});
expect(selectAll).toHaveTextContent(/deselect all/i);
});
3 changes: 2 additions & 1 deletion public-dashboard-client/src/controls/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ export const DropdownMenuItem = styled.div`
*/
&:hover${(props) =>
props.highlightedSelector ? `, ${props.highlightedSelector}` : ""} {
background: ${(props) => props.theme.colors.highlight};
background: ${(props) =>
props.highlightColor || props.theme.colors.highlight};
color: ${(props) => props.theme.colors.bodyLight};
}
`;
Expand Down
37 changes: 33 additions & 4 deletions public-dashboard-client/src/utils/highlightFade.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,36 @@
// Recidiviz - a data platform for criminal justice reform
// Copyright (C) 2020 Recidiviz, Inc.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { color } from "d3-color";
import { interpolateRgb } from "d3-interpolate";
import { THEME } from "../theme";

const FADE_AMOUNT = 0.45;

export default function highlightFade(baseColor) {
const fadedColor = color(baseColor);
fadedColor.opacity = 0.45;
return fadedColor.toString();
export default function highlightFade(baseColor, { useOpacity = false } = {}) {
if (useOpacity) {
// in cases where we actually want the color to be transparent,
// this is a relatively straightforward opacity change
const fadedColor = color(baseColor);
fadedColor.opacity = 0.45;
return fadedColor.toString();
}
// in cases where we don't want a transparent color (which is most cases),
// this will create a tint ramp from background color to baseColor;
// the ramp goes from 0 to 1 with values analogous to opacity
return interpolateRgb(THEME.colors.background, baseColor)(FADE_AMOUNT);
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ export default function RecidivismRatesChart({ data, highlightedCohort }) {
fill: "none",
stroke:
highlighted && highlighted.label !== d.label
? highlightFade(d.color)
? // transparency helps this chart because the lines can overlap
highlightFade(d.color, { useOpacity: true })
: d.color,
strokeWidth: 2,
};
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4613,7 +4613,7 @@ d3-interpolate@1, d3-interpolate@^1.1.5:
dependencies:
d3-color "1"

"d3-interpolate@1.2.0 - 2":
"d3-interpolate@1.2.0 - 2", d3-interpolate@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/d3-interpolate/-/d3-interpolate-2.0.1.tgz#98be499cfb8a3b94d4ff616900501a64abc91163"
integrity sha512-c5UhwwTs/yybcmTpAVqwSFl6vrQ8JZJoT5F7xNFK9pymv5C0Ymcc9/LIJHtYIggg/yS9YHw8i8O8tgb9pupjeQ==
Expand Down

0 comments on commit 35b171e

Please sign in to comment.