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.

Sign up for GitHub

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

Filter by Date Pt. 2 - Custom date selection #3117

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
+800 −72
Open

Filter by Date Pt. 2 - Custom date selection #3117

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2649c0d
wip
angelathe Jan 7, 2025
c2a25ce
Display custom dates in filter date button when custom date selected
angelathe Jan 7, 2025
e611290
maintain state of custom dates by reading URL params
angelathe Jan 7, 2025
d92a516
set max value for start/end dates to today
angelathe Jan 8, 2025
5c1b5a7
move buildCustomDateRange logic to date-utils
angelathe Jan 8, 2025
64a27aa
fix reset state bugs when pressing clear on calendar and when start d…
angelathe Jan 8, 2025
016a23a
add JSdocs
angelathe Jan 9, 2025
61cbc3c
remove unused css, small fix in custom date button display
angelathe Jan 9, 2025
e718ac3
Add key to remove console warnings
angelathe Jan 9, 2025
bcb4606
update snapshot tests
angelathe Jan 9, 2025
39fd3e8
Merge branch 'main' into angela/2753-filter-by-date-p2
angelathe Jan 9, 2025
d634cc4
add dates to list of reset params
angelathe Jan 9, 2025
af0c329
clean up deleteQueryParam
angelathe Jan 9, 2025
343c944
add filter param names to an enum for safety/consistency
angelathe Jan 9, 2025
22d56d1
wip, refactoring param update functions
angelathe Jan 9, 2025
643400c
mock date for custom dates snapshot test
angelathe Jan 10, 2025
ec9a5fa
replace submit with clicking apply button
angelathe Jan 10, 2025
0b0792e
small update to paramKeys
angelathe Jan 10, 2025
432bd1e
Merge branch 'main' into angela/2753-filter-by-date-p2
angelathe Jan 10, 2025
0902cc2
abstract out RadioDateOption, and update snapshot tests
angelathe Jan 10, 2025
8f54b1a
Merge branch 'angela/2753-filter-by-date-p2' of https://github.com/CD…
angelathe Jan 10, 2025
5604ed3
end date no longer required, defaults to today
angelathe Jan 13, 2025
77fd9d0
fix reset from custom dates bug, old custom dates were still visible
angelathe Jan 13, 2025
cd4d82b
make isDefault false by default in updateQueryParam
angelathe Jan 13, 2025
a8c3aef
small nit fix for updatedParams decl
angelathe Jan 13, 2025
96387bb
abstract custom date input, add inline to FilterByDate, update snapshot
angelathe Jan 13, 2025
728618b
add RadioDateOptions for for rendering set of radio buttons
angelathe Jan 13, 2025
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
88 changes: 76 additions & 12 deletions containers/ecr-viewer/src/app/components/BaseFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import React, {
useEffect,
useRef,
} from "react";
import { Button } from "@trussworks/react-uswds";
import { Button, Label } from "@trussworks/react-uswds";
import { useRouter, usePathname, useSearchParams } from "next/navigation";
import { FILTER_CLOSED, FILTER_SUBMITTED, FilterOpenContext } from "./Filters";

Expand All @@ -24,8 +24,7 @@ export const useQueryParam = () => {

// Set a query param with a specific value
const setQueryParam = useCallback(
(key: string, value: string) => {
const params = new URLSearchParams(searchParams.toString());
(params: URLSearchParams, key: string, value: string) => {
params.set("page", "1");
params.set(key, value);
return params;
Expand All @@ -35,10 +34,9 @@ export const useQueryParam = () => {

// Delete a query param or a specific query param value
const deleteQueryParam = useCallback(
(name: string, value?: string) => {
const params = new URLSearchParams(searchParams.toString());
(params: URLSearchParams, key: string) => {
params.set("page", "1");
params.delete(name, value);
params.delete(key);
return params;
},
[searchParams],
Expand All @@ -52,6 +50,7 @@ export const useQueryParam = () => {

// Update a specific query param (set or delete if default)
const updateQueryParam = (
params: URLSearchParams,
key: string,
value: string | { [key: string]: boolean },
isDefault: boolean,
Expand All @@ -62,14 +61,13 @@ export const useQueryParam = () => {
.join("|");
}
const updatedParams = isDefault
? deleteQueryParam(key)
: setQueryParam(key, value);
? deleteQueryParam(params, key)
: setQueryParam(params, key, value);

// Update query params
pushQueryUpdate(updatedParams, [key]);
return updatedParams;
};

return { searchParams, updateQueryParam, pushQueryUpdate };
return { searchParams, deleteQueryParam, updateQueryParam, pushQueryUpdate };
};

/**
Expand Down Expand Up @@ -210,7 +208,6 @@ const FilterLegend = ({ type }: { type: string }) => {
const ApplyFilterButton = ({ type }: { type: string }) => {
return (
<div className="display-flex flex-column flex-stretch padding-x-105">
<div className="border-top-1px border-base-lighter margin-x-neg-105"></div>
<Button
type="submit"
className="margin-y-1 margin-x-0 padding-y-1 padding-x-205 flex-fill text-no-wrap"
Expand All @@ -221,3 +218,70 @@ const ApplyFilterButton = ({ type }: { type: string }) => {
</div>
);
};

/**
* A custom date input component for selecting a start date and an end date.
* @param props -
* @param props.setStartDate - Function to update the start date state
* @param props.setEndDate - Function to update the end date state
* @param props.startDate - Current start date value
* @param props.endDate - Current end date value
* @returns A JSX element containing two date input fields for start and end dates.
*/
export const CustomDateInput = ({
setStartDate,
setEndDate,
startDate,
endDate,
}: {
setStartDate: (date: string) => void;
setEndDate: (date: string) => void;
startDate: string;
endDate: string;
}) => {
return (
<div className="padding-x-105">
<div>
<Label htmlFor="start-date" className="margin-top-1">
angelathe marked this conversation as resolved.
Show resolved Hide resolved
Start date
</Label>
<input
key="1"
id="start-date"
data-testid="start-date"
type="date"
className="usa-input width-card margin-top-0 border-base-dark custom-date"
defaultValue={startDate || ""}
max={new Date().toISOString().split("T")[0]}
required
aria-label="Start Date"
onChange={(e) => {
const date = e.target.value;
setStartDate(date);
}}
/>
</div>

<div>
<Label htmlFor="end-date" className="margin-top-1">
End date
</Label>
<input
key="2"
id="end-date"
data-testid="end-date"
type="date"
className="usa-input width-card margin-top-0 border-base-dark custom-date"
defaultValue={endDate || ""}
required
angelathe marked this conversation as resolved.
Show resolved Hide resolved
max={new Date().toISOString().split("T")[0]}
aria-label="End Date"
onChange={(e) => {
const date = e.target.value;
setEndDate(date);
}}
/>
</div>
</div>
);
};
169 changes: 137 additions & 32 deletions containers/ecr-viewer/src/app/components/Filters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,23 @@ import React, {
useState,
} from "react";
import { Button, Icon } from "@trussworks/react-uswds";
import { useQueryParam, Filter } from "./BaseFilter";
import {
useQueryParam,
Filter,
CustomDateInput,
} from "@/app/components/BaseFilter";
import {
DEFAULT_DATE_RANGE,
DateRangeOptions,
dateRangeLabels,
} from "@/app/view-data/utils/date-utils";
import { formatDateTime } from "@/app/services/formatService";

enum ParamName {
Condition = "condition",
DateRange = "dateRange",
Dates = "dates",
}

// We use a context to communicate between the overall <Filters /> component
// and the `<Filter />` component to avoid prop drilling
Expand Down Expand Up @@ -77,7 +88,7 @@ const Filters = () => {
}
}, [filterBoxOpen]);

const paramKeys = ["condition", "dateRange"];
const paramKeys = [ParamName.Condition, ParamName.DateRange, ParamName.Dates];
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this instead be computed from the enum? (is a formal enum worth it or just use a plain object?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just made an update!

(Maybe an enum is overkill, but thought it would make things clearer in case we expand the scope of filters?)

const resetToDefault = () => {
const params = new URLSearchParams(searchParams.toString());
params.set("page", "1");
Expand Down Expand Up @@ -124,7 +135,8 @@ const Filters = () => {
* - Updates the browser's query string when the filter is applied.
*/
const FilterReportableConditions = () => {
const { searchParams, updateQueryParam } = useQueryParam();
const { searchParams, updateQueryParam, pushQueryUpdate } = useQueryParam();
const params = new URLSearchParams(searchParams.toString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to do this copy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to pass params to updateQueryParam in the submitHandler (can't pass searchParams which is read-only)

Could move Line 139 into the submitHandler so it's more clear why that's needed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd move this into updateQueryParams so it always returns a new params and doesn't mutate. Especially since mutating state can be such a footgun, best to avoid it entirely by default

const [filterConditions, setFilterConditions] = useState<{
[key: string]: boolean;
}>({});
Expand Down Expand Up @@ -174,7 +186,7 @@ const FilterReportableConditions = () => {
);

const resetFilterConditions = (conditions: string[]) => {
const conditionParam = searchParams.get("condition");
const conditionParam = searchParams.get(ParamName.Condition);
const conditionsToTrue = new Set(conditionParam?.split("|") || []);

const conditionValue = (c: string) => {
Expand Down Expand Up @@ -211,9 +223,15 @@ const FilterReportableConditions = () => {
(key) => filterConditions[key] === true,
).length || "0"
}
submitHandler={() =>
updateQueryParam("condition", filterConditions, isAllSelected)
}
submitHandler={() => {
const updatedParams = updateQueryParam(
params,
ParamName.Condition,
filterConditions,
isAllSelected,
);
pushQueryUpdate(updatedParams, [ParamName.Condition]);
}}
>
{/* Select All checkbox */}
<div className="display-flex flex-column">
Expand Down Expand Up @@ -260,6 +278,7 @@ const FilterReportableConditions = () => {
))}
</div>
</div>
<div className="border-top-1px border-base-lighter margin-x-neg-105"></div>
</Filter>
);
};
Expand All @@ -271,10 +290,13 @@ const FilterReportableConditions = () => {
* - Updates the browser's query string when the filter is applied.
*/
const FilterByDate = () => {
const { searchParams, updateQueryParam } = useQueryParam();
const { searchParams, deleteQueryParam, updateQueryParam, pushQueryUpdate } =
useQueryParam();

const [filterDateOption, setFilterDateOption] =
useState<string>(DEFAULT_DATE_RANGE);
const [startDate, setStartDate] = useState<string>("");
const [endDate, setEndDate] = useState<string>("");
const isFilterDateDefault = filterDateOption === DEFAULT_DATE_RANGE;

const handleDateOptionChange = (
Expand All @@ -286,52 +308,135 @@ const FilterByDate = () => {
};

const resetFilterDate = () => {
angelathe marked this conversation as resolved.
Show resolved Hide resolved
const queryDateRange = searchParams.get("dateRange");
const queryDateRange = searchParams.get(ParamName.DateRange);
Copy link
Collaborator

Choose a reason for hiding this comment

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

if a user manually deletes the dates param, it ends up looking like this

image

should we reset to the default instead at this point? cc @ashton-skylight

if (!queryDateRange) {
setFilterDateOption(DEFAULT_DATE_RANGE);
} else if (queryDateRange !== filterDateOption) {
setFilterDateOption(queryDateRange);
}
if (queryDateRange === "custom") {
const queryCustomDates = searchParams.get(ParamName.Dates) || "";
const [startDate, endDate] = queryCustomDates?.split("|");
setStartDate(startDate);
setEndDate(endDate);
}
};

// on mount, make sure the filters match the search params
useEffect(resetFilterDate, []);

const RadioDateOption = ({
angelathe marked this conversation as resolved.
Show resolved Hide resolved
option,
label,
classNames,
}: {
option: string;
label: string;
classNames?: string;
}) => {
return (
<div
className={`checkbox-color usa-radio padding-x-105 ${classNames}`}
key={`filter-date-${option}`}
>
<input
id={`filter-date-${option}`}
className="usa-radio__input"
type="radio"
name="filterDateOptions"
value={option}
onChange={handleDateOptionChange}
checked={filterDateOption === option}
/>
<label
className="line-height-sans-6 font-sans-xs margin-y-0 usa-radio__label text-no-wrap"
htmlFor={`filter-date-${option}`}
>
{label}
</label>
</div>
);
};

const submitHandler = () => {
const params = new URLSearchParams(searchParams.toString());
if (filterDateOption === "custom") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what were the pros/cons/thinking on the two param approach vs a single param like custom(start,end)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, missed this comment on Friday! My thoughts are below, and I'm definitely open to changing the approach.

I went with two URL parameters to keep things simpler. One shows that it’s a custom date, and the other holds the actual dates. Keeping these definitions separate means we don’t have to deal with extra parsing every time we just need one of them (felt cleaner and easier to work with this approach). Also, having two separate parameters helps keep the radio button option name and the URL param values in sync, which makes it simpler to match what the user picks to what’s in the query string.

Also, keeping custom dates in their own parameter seemed like it would make it easier to extend things in the future—like if we wanted to add time ranges. Saves us from having to dig through and parse a single dateRange parameter if we need to add more customizations down the line.

The main con with maintaining the two parameters is maintaining consistency between dateRange and dates, i.e. making sure that dates only exists and is processed when dateRange=custom, and that dates does not exist when dateRange is set to one of the other predefined date ranges. Using the single dateRange parameter also to house the custom dates would eliminate this risk.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That matches what I see as well. We just need to make sure we've got robust handling of the keeping in sync and edge cases there

// TODO ANGELA: Temp: Placeholder errorhandling for when Start Date > End Date
if (new Date(startDate) > new Date(endDate)) {
alert(
"Invalid date range. Start date must be earlier or equal to the end date.",
);
resetFilterDate();
return;
}
Comment on lines +336 to +342
Copy link
Collaborator Author

@angelathe angelathe Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Note: This is temporary -- a placeholder alert to safeguard against error where Start date > end date. Still working on how to make the custom error tooltip for this / might get brought out into a separate small ticket

const datesParam = `${startDate}|${endDate}`;
let updatedParams: URLSearchParams;
updatedParams = updateQueryParam(
angelathe marked this conversation as resolved.
Show resolved Hide resolved
params,
ParamName.DateRange,
filterDateOption,
isFilterDateDefault,
);
updatedParams = updateQueryParam(
updatedParams,
ParamName.Dates,
datesParam,
false,
angelathe marked this conversation as resolved.
Show resolved Hide resolved
);
pushQueryUpdate(updatedParams, [ParamName.DateRange, ParamName.Dates]);
} else {
let updatedParams: URLSearchParams;
updatedParams = updateQueryParam(
params,
ParamName.DateRange,
filterDateOption,
isFilterDateDefault,
);
updatedParams = deleteQueryParam(updatedParams, ParamName.Dates);
pushQueryUpdate(updatedParams, [ParamName.DateRange, ParamName.Dates]);
setStartDate("");
setEndDate("");
}
};

return (
<Filter
type="Received Date"
isActive={true}
resetHandler={resetFilterDate}
icon={Icon.Event}
title={dateRangeLabels[filterDateOption as DateRangeOptions] || ""}
submitHandler={() =>
updateQueryParam("dateRange", filterDateOption, isFilterDateDefault)
title={
filterDateOption === "custom"
? startDate && endDate
? `From ${formatDateTime(startDate)} to ${formatDateTime(endDate)}`
: ""
: dateRangeLabels[filterDateOption as DateRangeOptions] || ""
}
submitHandler={submitHandler}
>
<div className="display-flex flex-column">
{Object.values(DateRangeOptions).map((option) => (
<div
className="checkbox-color usa-radio padding-bottom-1 padding-x-105"
<RadioDateOption
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe move this abstraction up one level to RadioDateOptions? there's still a bit of "guts" in here that could have a nicer API if it handles the options bit. Then it could just pass what are all the options handler for option change and current option, which is pretty universal to any set of radio buttons

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mcmcgrath13 Made these changes in the last commit if you want to take a look -- but I'm wondering if it's really necessary for this ticket?

I think I still need to keep RadioDateOption because of the lone custom date radio option, but then adding RadioDateOptions feels redundant since it's essentially just passing the same parameters down to RadioDateOption. (it's just adding an extra layer....?)

My preference would be to rollback that commit, but lmk if you have any thoughts on changes I could make so RadioDateOptions actually simplifies the logic.

key={`filter-date-${option}`}
>
<input
id={`filter-date-${option}`}
className="usa-radio__input"
type="radio"
name="filterDateOptions"
value={option}
onChange={handleDateOptionChange}
checked={filterDateOption === option}
/>
<label
className="line-height-sans-6 font-sans-xs margin-y-0 usa-radio__label text-no-wrap"
htmlFor={`filter-date-${option}`}
>
{dateRangeLabels[option]}
</label>
</div>
option={option}
label={dateRangeLabels[option]}
classNames="padding-bottom-1"
/>
))}
<div className="border-top-1px border-base-lighter margin-x-105"></div>
<div className="border-top-1px border-base-lighter margin-x-105 padding-bottom-1"></div>
<RadioDateOption
key="filter-date-custom"
option="custom"
label="Custom date range"
/>
{filterDateOption === "custom" && (
<CustomDateInput
setStartDate={setStartDate}
setEndDate={setEndDate}
startDate={startDate}
endDate={endDate}
/>
)}
</div>
</Filter>
);
Expand Down
Loading
Loading