Skip to content

Commit

Permalink
Multiselect rewrite (#743)
Browse files Browse the repository at this point in the history
The original multiselect library I started with for filters has turned out to be not that great - it was not originally written in typescript, it's difficult to customize, and it's not accessible for screen readers. So after struggling to add screen-reader accessibility to the existing component, I've just decided to use the much better react-select library and rewrite our custom multiselect filter.

react-select is written in typescript, has great accessibility features (nicely handles keyboard navigation for search-bar/option-list/selected-list, and has great default screen reader setting plus customization options), and it has a nice system for customizing by overwriting and recomposing its subcomponents. (The entire MultiSelect.tsx file is new, so you can just review that file alone and ignore the diff)

Since I then needed to update a lot of the filter styles for the new component, I ended up doing a much needed refactor of all the PortfolioFilters styles. I pulled out the minmax-select and multi-select styles into their one contained component styles, and also tried to simplify the organization of desktop/mobile styles to keep things close together (rather than the mostly separate mobile-specific styles nested in it's own section. I also tried to pull other changes out to component styles (Alerts, Buttons), and address some other issues at the same time.

While I was separating out the minmax-select component styles I decided to pull the component into it's own file as well (the contents haven't changed).

This all addresses a few outstanding tickets:

multiselect now works with screen-readers [sc-12099]
outline replaced with border for elements with border-radius [sc-11773]
fixes multiselect searchbar placeholder text alignment [sc-12186]
center align apply buttons on mobile [sc-12060]
  • Loading branch information
austensen authored Apr 11, 2023
1 parent e93ad49 commit b85c223
Show file tree
Hide file tree
Showing 18 changed files with 1,247 additions and 1,463 deletions.
9 changes: 5 additions & 4 deletions client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"@lingui/macro": "^2.9.1",
"@lingui/react": "^2.9.1",
"@tanstack/react-table": "8.5.30",
"@typeform/embed-react": "^1.20.0",
"@types/chartjs-plugin-annotation": "^0.5.0",
"@types/file-saver": "^2.0.1",
"@types/googlemaps": "^3.0.0",
Expand All @@ -32,7 +33,6 @@
"@types/react-modal": "^3.8.2",
"@types/react-router-dom": "5.1.5",
"@types/react-transition-group": "^4.4.0",
"@typeform/embed-react": "^1.20.0",
"@xstate/react": "^0.8.1",
"algoliasearch": "^4.10.5",
"babel-core": "^7.0.0-bridge.0",
Expand All @@ -50,7 +50,6 @@
"leaflet": "^1.1.0",
"lodash": "^4.17.21",
"mapbox-gl": "^1.12.0",
"sass": "^1.49.9",
"prop-types": "^15.7.2",
"react": "^16.11.0",
"react-app-polyfill": "^0.2.0",
Expand All @@ -69,9 +68,11 @@
"react-papaparse": "^3.17.1",
"react-router-dom": "^5.1.2",
"react-scripts": "^4.0.3",
"react-select": "^5.7.2",
"react-social": "^1.10.0",
"react-transition-group": "^2.2.1",
"rollbar": "^2.14.4",
"sass": "^1.49.9",
"spectre.scss": "0.0.2",
"typescript": "^4.9.3",
"widont": "^0.3.3",
Expand All @@ -97,11 +98,11 @@
"@babel/preset-env": "^7.7.6",
"@babel/preset-typescript": "^7.7.4",
"@babel/register": "^7.7.4",
"@svgr/webpack": "5.5.0",
"dotenv": "^8.2.0",
"jest-fetch-mock": "^3.0.3",
"npm-run-all": "^4.0.2",
"prettier": "2.0.5",
"@svgr/webpack": "5.5.0"
"prettier": "2.0.5"
},
"overrides": {
"@svgr/webpack": "$@svgr/webpack"
Expand Down
2 changes: 0 additions & 2 deletions client/src/components/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import React, { forwardRef, useLayoutEffect, useState } from "react";
import { CloseButton } from "./CloseButton";
import "styles/_alert.scss";

// TODO: Check with design if we are going to want the icons matching type

export interface AlertProps extends React.ComponentPropsWithoutRef<"div"> {
children: React.ReactNode;
type?: "error" | "success" | "info";
Expand Down
128 changes: 128 additions & 0 deletions client/src/components/MinMaxSelect.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import React from "react";
import { Trans } from "@lingui/macro";
import classnames from "classnames";
import helpers from "util/helpers";
import { Alert } from "./Alert";
import { FilterNumberRange, MINMAX_DEFAULT } from "./PropertiesList";

function MinMaxSelect(props: {
options: FilterNumberRange;
onApply: (selectedList: FilterNumberRange) => void;
id?: string;
onFocusInput?: () => void;
}) {
const { options, onApply, id, onFocusInput } = props;
const [minMax, setMinMax] = React.useState<FilterNumberRange>(MINMAX_DEFAULT);
const [minMaxErrors, setMinMaxErrors] = React.useState<MinMaxErrors>([false, false, undefined]);

return (
<form id={id} className="minmax-container">
{minMaxErrors[0] || minMaxErrors[1] ? (
<div className="alerts-container">
<Alert type="error" variant="primary" closeType="none" role="status">
{minMaxErrors[2]}
</Alert>
</div>
) : (
<></>
)}
<div className="label-input-container">
<div className="labels-container">
<label htmlFor="min-input">
<Trans>MIN</Trans>
</label>
<label htmlFor="max-input">
<Trans>MAX</Trans>
</label>
</div>
<div className="inputs-container">
<input
id={`${id || "minmax-select"}_min-input`}
type="number"
min={options[0]}
max={options[1]}
value={minMax[0] == null ? "" : minMax[0]}
onKeyDown={helpers.preventNonNumericalInput}
onChange={(e) => {
setMinMaxErrors([false, false, undefined]);
setMinMax([cleanNumberInput(e.target.value), minMax[1]]);
}}
onFocus={onFocusInput}
className={classnames("min-input", { hasError: minMaxErrors[0] })}
/>
<span>
<Trans>and</Trans>
</span>
<input
id={`${id || "minmax-select"}_max-input`}
type="number"
min={options[0]}
max={options[1]}
value={minMax[1] == null ? "" : minMax[1]}
onKeyDown={helpers.preventNonNumericalInput}
onChange={(e) => {
setMinMaxErrors([false, false, undefined]);
setMinMax([minMax[0], cleanNumberInput(e.target.value)]);
}}
onFocus={onFocusInput}
className={classnames("max-input", { hasError: minMaxErrors[1] })}
/>
</div>
</div>
<button
onClick={(e) => {
e.preventDefault();
const errors = minMaxHasError(minMax, options);
if (errors[0] || errors[1]) {
setMinMaxErrors(errors);
} else {
onApply(minMax);
}
}}
className="button is-primary"
>
<Trans>Apply</Trans>
</button>
</form>
);
}

function cleanNumberInput(value: string): number | undefined {
if (!new RegExp("[0-9+]").test(value)) return undefined;
return Number(value);
}

type MinMaxErrors = [boolean, boolean, JSX.Element | undefined];
function minMaxHasError(values: FilterNumberRange, options: FilterNumberRange): MinMaxErrors {
const [minValue, maxValue] = values;
const [minOption, maxOption] = options;
let minHasError = false;
let maxHasError = false;
let errorMessage: JSX.Element | undefined;

if (minValue != null && maxValue != null && minValue > maxValue) {
minHasError = true;
maxHasError = true;
errorMessage = <Trans>Min must be less than or equal to Max</Trans>;
}
if (minValue != null && minValue < 0) {
minHasError = true;
errorMessage = <Trans>Min must be greater than 0</Trans>;
}
if (maxValue != null && maxValue < 0) {
minHasError = true;
errorMessage = <Trans>Max must be greater than 0</Trans>;
}
if (maxValue != null && minOption != null && maxValue < minOption) {
maxHasError = true;
errorMessage = <Trans>Max must be greater than {minOption}</Trans>;
}
if (minValue != null && maxOption != null && minValue > maxOption) {
minHasError = true;
errorMessage = <Trans>Min must be less than {maxOption}</Trans>;
}

return [minHasError, maxHasError, errorMessage];
}

export default MinMaxSelect;
Loading

0 comments on commit b85c223

Please sign in to comment.