Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recreate MetricsView Component for Data Visualization #101

Merged
merged 5 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions publisher/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { trackNavigation } from "./analytics";
import { DataUpload } from "./components/DataUpload";
import { PageWrapper } from "./components/Forms";
import Header from "./components/Header";
import { MetricsView } from "./components/MetricConfiguration/MetricsView";
import CreateReports from "./components/Reports/CreateReport";
import ReportDataEntry from "./components/Reports/ReportDataEntry";
import ReviewMetrics from "./components/ReviewMetrics/ReviewMetrics";
Expand All @@ -42,6 +43,7 @@ const App: React.FC = (): ReactElement => {

<Routes>
<Route path="/" element={<Reports />} />
<Route path="/data" element={<MetricsView />} />
<Route path="/reports/create" element={<CreateReports />} />
<Route path="/reports/:id" element={<ReportDataEntry />} />
<Route path="/settings" element={<Settings />} />
Expand Down
11 changes: 11 additions & 0 deletions publisher/src/components/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ enum MenuItems {
LearnMore = "LEARN MORE",
Settings = "SETTINGS",
Agencies = "AGENCIES",
Data = "DATA",
}

const Menu = () => {
Expand Down Expand Up @@ -76,6 +77,8 @@ const Menu = () => {
setActiveMenuItem(MenuItems.CreateReport);
} else if (location.pathname === "/settings") {
setActiveMenuItem(MenuItems.Settings);
} else if (location.pathname === "/data") {
setActiveMenuItem(MenuItems.Data);
} else {
setActiveMenuItem(undefined);
}
Expand All @@ -97,6 +100,14 @@ const Menu = () => {
Reports
</MenuItem>

{/* Data (Visualizations) */}
<MenuItem
onClick={() => navigate("/data")}
active={activeMenuItem === MenuItems.Data}
>
Data
</MenuItem>

{/* Learn More */}
<MenuItem active={activeMenuItem === MenuItems.LearnMore}>
<a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export const MetricsViewControlPanel = styled.div`
overflow-y: scroll;
`;

export const MetricsViewControlPanelOverflowHidden = styled(
MetricsViewControlPanel
)`
overflow-y: hidden;
`;

export const PanelContainerLeft = styled.div`
width: 35%;
height: 100%;
Expand All @@ -54,6 +60,7 @@ export const PanelContainerRight = styled.div`
display: flex;
position: relative;
flex-direction: column;
overflow-y: scroll;
`;

type MetricBoxContainerProps = {
Expand All @@ -79,6 +86,15 @@ export const MetricBoxContainer = styled.div<MetricBoxContainerProps>`
}
`;

export const MetricViewBoxContainer = styled(MetricBoxContainer)<{
selected?: boolean;
}>`
max-width: 100%;
min-height: 50px;
border: ${({ selected }) => selected && `1px solid ${palette.solid.blue}`};
margin-bottom: 5px;
`;

export const MetricBoxWrapper = styled.div`
display: flex;
`;
Expand All @@ -88,6 +104,7 @@ export const ActiveMetricSettingHeader = styled.div`
z-index: 1;
background: ${palette.solid.white};
padding: 10px 15px 0 15px;
margin-bottom: 20px;
`;

export const MetricNameBadgeToggleWrapper = styled.div`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export type MetricSettings = {
}[];
};

type MetricSettingsObj = {
export type MetricSettingsObj = {
[key: string]: MetricType;
};

Expand Down
265 changes: 265 additions & 0 deletions publisher/src/components/MetricConfiguration/MetricsView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
// Recidiviz - a data platform for criminal justice reform
// Copyright (C) 2022 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 { reaction, when } from "mobx";
import { observer } from "mobx-react-lite";
import React, { useEffect, useState } from "react";

import { Metric, ReportFrequency } from "../../shared/types";
import { useStore } from "../../stores";
import { removeSnakeCase } from "../../utils";
import { Badge, BadgeColorMapping } from "../Badge";
import ConnectedDatapointsView from "../DataViz/DatapointsView";
import { NotReportedIcon } from "../Forms";
import { Loading } from "../Loading";
import { PageTitle, TabbedBar, TabbedItem, TabbedOptions } from "../Reports";
import {
ActiveMetricSettingHeader,
MetricBoxWrapper,
MetricDescription,
MetricName,
MetricNameBadgeToggleWrapper,
MetricNameBadgeWrapper,
MetricSettingsObj,
MetricsViewContainer,
MetricsViewControlPanelOverflowHidden,
MetricViewBoxContainer,
PanelContainerLeft,
PanelContainerRight,
} from ".";

type MetricBoxProps = {
metricKey: string;
displayName: string;
frequency: ReportFrequency;
description: string;
enabled?: boolean;
activeMetricKey: string;
setActiveMetricKey: React.Dispatch<React.SetStateAction<string>>;
};

const reportFrequencyBadgeColors: BadgeColorMapping = {
ANNUAL: "ORANGE",
MONTHLY: "GREEN",
};

const MetricBox: React.FC<MetricBoxProps> = ({
metricKey,
displayName,
frequency,
description,
enabled,
activeMetricKey,
setActiveMetricKey,
}): JSX.Element => {
return (
<MetricViewBoxContainer
onClick={() => setActiveMetricKey(metricKey)}
enabled={enabled}
selected={metricKey === activeMetricKey}
>
<MetricNameBadgeToggleWrapper>
<MetricNameBadgeWrapper>
<MetricName>{displayName}</MetricName>
<Badge
color={reportFrequencyBadgeColors[frequency]}
disabled={!enabled}
>
{frequency}
</Badge>
</MetricNameBadgeWrapper>

{!enabled && <NotReportedIcon noTooltip />}
</MetricNameBadgeToggleWrapper>

<MetricDescription>{description}</MetricDescription>
</MetricViewBoxContainer>
);
};

export const MetricsView: React.FC = observer(() => {
const { reportStore, userStore, datapointsStore } = useStore();

const [activeMetricFilter, setActiveMetricFilter] = useState<string>();
const [isLoading, setIsLoading] = useState<boolean>(true);
const [loadingError, setLoadingError] = useState<string | undefined>(
undefined
);
const [activeMetricKey, setActiveMetricKey] = useState<string>("");
const [metricSettings, setMetricSettings] = useState<{
[key: string]: Metric;
}>({});

const filteredMetricSettings: MetricSettingsObj = Object.values(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we share the logic between here and MetricsConfiguration?

metricSettings
)
.filter(
(metric) =>
metric.system.toLowerCase() === activeMetricFilter?.toLowerCase()
)
?.reduce((res: MetricSettingsObj, metric) => {
res[metric.key] = metric;
return res;
}, {});

const fetchAndSetReportSettings = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with fetchAndSetReportSettings? Would it be sufficient to share these functions, to avoid going the full route of creating a mobx store for metric settings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though w a mobx store or some sort of global state, we could avoid having to re-fetch metric settings every time the user enters / leaves one of these pages

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree with @terryttsai here actually ... it seems worthwhile creating some sort of global state for this metric information, since we're now using it in two different views (do we also use it on Data Entry or is it not needed there?) and will likely use it in additional views later on. Can definitely be done in a followup PR, it's not urgent -- we can taskify for now and come back to it at the end of the sprint if there's time. In the meantime, I like the suggestion of factoring out the shared code!

Also, unrelated, but can we rename it? reportSettings confused me -- they're metric settings, not report settings, right? And really it's more like metric metadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good points! I'll taskify and work on it in a follow up PR.

since we're now using it in two different views (do we also use it on Data Entry or is it not needed there?)

Unless I'm mistaken, we don't use this function or get any settings in the data entry.

Also, unrelated, but can we rename it? reportSettings confused me -- they're metric settings, not report settings, right? And really it's more like metric metadata?

Haha - yeah, I guess they are technically metric settings for a report? Maybe that's why we went with that in the beginning. I'm good with fetchAndSetMetricSettings.

const response = (await reportStore.getReportSettings()) as
| Response
| Error;

setIsLoading(false);

if (response instanceof Error) {
return setLoadingError(response.message);
}

const reportSettings = (await response.json()) as Metric[];
const metricKeyToMetricMap: { [key: string]: Metric } = {};

reportSettings?.forEach((metric) => {
metricKeyToMetricMap[metric.key] = metric;
});

setMetricSettings(metricKeyToMetricMap);
setActiveMetricKey(Object.keys(metricKeyToMetricMap)[0]);
};

useEffect(
() =>
// return when's disposer so it is cleaned up if it never runs
when(
() => userStore.userInfoLoaded,
async () => {
fetchAndSetReportSettings();
datapointsStore.getDatapoints();
setActiveMetricFilter(
removeSnakeCase(userStore.currentAgency?.systems[0] as string)
);
}
),
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
);

// reload report overviews when the current agency ID changes
useEffect(
() =>
// return disposer so it is cleaned up if it never runs
reaction(
() => userStore.currentAgencyId,
async (currentAgencyId, previousAgencyId) => {
// prevents us from calling getDatapoints twice on initial load
if (previousAgencyId !== undefined) {
setIsLoading(true);
fetchAndSetReportSettings();
await datapointsStore.getDatapoints();
setActiveMetricFilter(
removeSnakeCase(userStore.currentAgency?.systems[0] as string)
);
}
}
),
// eslint-disable-next-line react-hooks/exhaustive-deps
[userStore]
);

useEffect(
() => {
setActiveMetricKey(Object.keys(filteredMetricSettings)[0]);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[activeMetricFilter]
);

if (isLoading) {
return <Loading />;
}

if (!metricSettings[activeMetricKey]) {
return <div>Error: {loadingError}</div>;
}

return (
<>
<MetricsViewContainer>
<PageTitle>Metrics</PageTitle>

<TabbedBar>
<TabbedOptions>
{userStore.currentAgency?.systems.map((filterOption) => (
<TabbedItem
key={filterOption}
selected={activeMetricFilter === removeSnakeCase(filterOption)}
onClick={() =>
setActiveMetricFilter(removeSnakeCase(filterOption))
}
capitalize
>
{removeSnakeCase(filterOption.toLowerCase())}
</TabbedItem>
))}
</TabbedOptions>
</TabbedBar>

<MetricsViewControlPanelOverflowHidden>
{/* List Of Metrics */}
<PanelContainerLeft>
{filteredMetricSettings &&
Object.values(filteredMetricSettings).map((metric) => (
<MetricBoxWrapper key={metric.key}>
<MetricBox
metricKey={metric.key}
displayName={metric.display_name}
frequency={metric.frequency as ReportFrequency}
description={metric.description}
enabled={metric.enabled}
activeMetricKey={activeMetricKey}
setActiveMetricKey={setActiveMetricKey}
/>
</MetricBoxWrapper>
))}
</PanelContainerLeft>

{/* Data Visualization */}
<PanelContainerRight>
<ActiveMetricSettingHeader>
<MetricNameBadgeWrapper>
<MetricName isTitle>
{filteredMetricSettings[activeMetricKey]?.display_name}
</MetricName>
{filteredMetricSettings[activeMetricKey]?.frequency && (
<Badge
color={
reportFrequencyBadgeColors[
filteredMetricSettings[activeMetricKey]
.frequency as ReportFrequency
]
}
>
{filteredMetricSettings[activeMetricKey].frequency}
</Badge>
)}
</MetricNameBadgeWrapper>
</ActiveMetricSettingHeader>

<ConnectedDatapointsView metric={activeMetricKey} />
</PanelContainerRight>
</MetricsViewControlPanelOverflowHidden>
</MetricsViewContainer>
</>
);
});