From f523b280b876ff1ddd1cb4b39ff12fd26c840a02 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 15 Oct 2024 10:10:34 -0500 Subject: [PATCH 1/5] style: UX Review fixes --- .../components/UI/FilterTags/FilterTags.jsx | 52 +++++++++++++------ frontend/src/constants.js | 2 +- .../list/CANFilterButton/CANFiilterButton.jsx | 6 +-- .../src/pages/cans/list/CanList.helpers.js | 10 ++-- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/frontend/src/components/UI/FilterTags/FilterTags.jsx b/frontend/src/components/UI/FilterTags/FilterTags.jsx index 73d4a9cda4..c743594363 100644 --- a/frontend/src/components/UI/FilterTags/FilterTags.jsx +++ b/frontend/src/components/UI/FilterTags/FilterTags.jsx @@ -2,26 +2,41 @@ import icons from "../../../uswds/img/sprite.svg"; import Tag from "../Tag"; /** - * A filter tags. + * @typedef {Object} Tag + * @property {string} filter - The filter associated with the tag + * @property {string} tagText - The text displayed on the tag + */ + +/** + * @description - A component to display a filter tag. + * @component + * @param {Object} props - The component props. + * @param {Tag} props.tag - The tag to display. + * @param {Function} props.removeFilter - A function to call to remove the tag. + * @returns {JSX.Element} - The filter tag component. (A pill with an 'x' to remove it) + */ +const FilterTag = ({ tag, removeFilter }) => ( + + {tag.tagText} + removeFilter(tag)} + id={`filter-tag-${tag.filter}`} + > + + + +); + +/** + * @description - A component to display filter tags. + * @component * @param {Object} props - The component props. * @param {Function} props.removeFilter - A function to call to remove a filter/tag. - * @param {string[]} props.tagsList - An array of tags to display. + * @param {Tag[]} props.tagsList - An array of tags to display. * @returns {JSX.Element} - The filter tags component. (Pills with an 'x' to remove them) */ -export const FilterTags = ({ removeFilter, tagsList }) => { - const FilterTag = ({ tag }) => ( - - {tag.tagText} - removeFilter(tag)} - id={`filter-tag-${tag.filter}`} - > - - - - ); - +const FilterTags = ({ tagsList, removeFilter }) => { return (
{tagsList.map((tag, index) => { @@ -30,7 +45,10 @@ export const FilterTags = ({ removeFilter, tagsList }) => { key={index} className="padding-right-205 padding-top-05" > - + ); })} diff --git a/frontend/src/constants.js b/frontend/src/constants.js index ac8a1c97d2..72802bfc37 100644 --- a/frontend/src/constants.js +++ b/frontend/src/constants.js @@ -6,7 +6,7 @@ const constants = { const currentYear = currentDate.getFullYear(); const currentFiscalYear = currentMonth >= 9 ? currentYear + 1 : currentYear; const years = []; - for (let i = currentFiscalYear - 5; i <= currentFiscalYear + 5; i++) { + for (let i = currentFiscalYear + 5; i >= currentFiscalYear - 5; i--) { years.push(i); } return years; diff --git a/frontend/src/pages/cans/list/CANFilterButton/CANFiilterButton.jsx b/frontend/src/pages/cans/list/CANFilterButton/CANFiilterButton.jsx index 9d82dbdeda..be34469d18 100644 --- a/frontend/src/pages/cans/list/CANFilterButton/CANFiilterButton.jsx +++ b/frontend/src/pages/cans/list/CANFilterButton/CANFiilterButton.jsx @@ -31,7 +31,7 @@ export const CANFilterButton = ({ filters, setFilters, portfolioOptions }) => { activePeriod={activePeriod} setActivePeriod={setActivePeriod} legendClassname={legendStyles} - overrideStyles={{ width: "187px" }} + overrideStyles={{ width: "22.7rem" }} /> ,
{ transfer={transfer} setTransfer={setTransfer} legendClassname={legendStyles} - overrideStyles={{ width: "187px" }} + overrideStyles={{ width: "22.7rem" }} />
,
{ setPortfolio={setPortfolio} portfolioOptions={portfolioOptions} legendClassname={legendStyles} - overrideStyles={{ width: "187px" }} + overrideStyles={{ width: "22.7rem" }} />
]; diff --git a/frontend/src/pages/cans/list/CanList.helpers.js b/frontend/src/pages/cans/list/CanList.helpers.js index 1c38f9a7ca..f0ee8d1702 100644 --- a/frontend/src/pages/cans/list/CanList.helpers.js +++ b/frontend/src/pages/cans/list/CanList.helpers.js @@ -117,8 +117,10 @@ export const getPortfolioOptions = (cans) => { return acc; }, new Set()); - return Array.from(portfolios).map((portfolio, index) => ({ - id: index, - title: portfolio - })); + return Array.from(portfolios) + .sort((a, b) => a.localeCompare(b)) + .map((portfolio, index) => ({ + id: index, + title: portfolio + })); }; From e24dde5a94d74e17618901e31c5d7d1f3f29eda5 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 15 Oct 2024 10:22:56 -0500 Subject: [PATCH 2/5] test: cleanup test --- frontend/cypress/e2e/canList.cy.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frontend/cypress/e2e/canList.cy.js b/frontend/cypress/e2e/canList.cy.js index 2843b4a349..32c14f1320 100644 --- a/frontend/cypress/e2e/canList.cy.js +++ b/frontend/cypress/e2e/canList.cy.js @@ -75,11 +75,9 @@ describe("CAN List", () => { cy.get("span").contains("Direct").should("exist"); cy.get("span").contains("HMRF").should("exist"); - // check that the table is filtered correctly - // table should contain 6 rows - - cy.get("tbody").find("tr").should("have.length", 3); - + // No CANs found + cy.get("tbody").should("not.exist"); + cy.get("p.text-center").contains("No CANs found").should("exist"); // reset cy.get("button").contains("Filter").click(); cy.get("button").contains("Reset").click(); From 678bff9dac51f86ce30a0507fe148b07d2a21cef Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 15 Oct 2024 10:23:41 -0500 Subject: [PATCH 3/5] test: cleanup --- frontend/cypress/e2e/canList.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/cypress/e2e/canList.cy.js b/frontend/cypress/e2e/canList.cy.js index 32c14f1320..2b761b12a6 100644 --- a/frontend/cypress/e2e/canList.cy.js +++ b/frontend/cypress/e2e/canList.cy.js @@ -73,7 +73,7 @@ describe("CAN List", () => { cy.get("span").contains("1 Year").should("exist"); cy.get("span").contains("Direct").should("exist"); - cy.get("span").contains("HMRF").should("exist"); + cy.get("span").contains("CC").should("exist"); // No CANs found cy.get("tbody").should("not.exist"); From dfbcd538877bbf61f4346163ce17690739e1ba7c Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 15 Oct 2024 14:54:54 -0500 Subject: [PATCH 4/5] style: use name over abbreviation --- frontend/src/pages/cans/list/CanList.helpers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pages/cans/list/CanList.helpers.js b/frontend/src/pages/cans/list/CanList.helpers.js index f0ee8d1702..d37d8d2dbf 100644 --- a/frontend/src/pages/cans/list/CanList.helpers.js +++ b/frontend/src/pages/cans/list/CanList.helpers.js @@ -89,7 +89,7 @@ const applyAdditionalFilters = (cans, filters) => { } if (filters.portfolio && filters.portfolio.length > 0) { filteredCANs = filteredCANs.filter((can) => - filters.portfolio?.some((portfolio) => portfolio.title.toUpperCase() === can.portfolio.abbreviation) + filters.portfolio?.some((portfolio) => portfolio.title.toUpperCase() === can.portfolio.name.toUpperCase()) ); } // TODO: Add other filters here @@ -113,7 +113,7 @@ export const getPortfolioOptions = (cans) => { return []; } const portfolios = cans.reduce((acc, can) => { - acc.add(can.portfolio.abbreviation); + acc.add(can.portfolio.name); return acc; }, new Set()); From 1ad8bfbcacadeda972ba1854adf63b7f8fcef8f8 Mon Sep 17 00:00:00 2001 From: Frank Pigeon Jr Date: Tue, 15 Oct 2024 17:15:01 -0500 Subject: [PATCH 5/5] style: moar UX feedback --- frontend/cypress/e2e/canList.cy.js | 2 +- .../components/UI/FilterTags/FilterTags.jsx | 5 +++-- .../src/pages/cans/list/CanList.helpers.js | 6 ++++-- .../pages/cans/list/CanList.helpers.test.js | 20 ++++++++++--------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/frontend/cypress/e2e/canList.cy.js b/frontend/cypress/e2e/canList.cy.js index 2b761b12a6..1ee0d0623c 100644 --- a/frontend/cypress/e2e/canList.cy.js +++ b/frontend/cypress/e2e/canList.cy.js @@ -73,7 +73,7 @@ describe("CAN List", () => { cy.get("span").contains("1 Year").should("exist"); cy.get("span").contains("Direct").should("exist"); - cy.get("span").contains("CC").should("exist"); + cy.get("span").contains("Child Care (CC)").should("exist"); // No CANs found cy.get("tbody").should("not.exist"); diff --git a/frontend/src/components/UI/FilterTags/FilterTags.jsx b/frontend/src/components/UI/FilterTags/FilterTags.jsx index c743594363..58783c261e 100644 --- a/frontend/src/components/UI/FilterTags/FilterTags.jsx +++ b/frontend/src/components/UI/FilterTags/FilterTags.jsx @@ -16,12 +16,13 @@ import Tag from "../Tag"; * @returns {JSX.Element} - The filter tag component. (A pill with an 'x' to remove it) */ const FilterTag = ({ tag, removeFilter }) => ( - + {tag.tagText} removeFilter(tag)} id={`filter-tag-${tag.filter}`} + style={{ fill: "currentColor" }} > diff --git a/frontend/src/pages/cans/list/CanList.helpers.js b/frontend/src/pages/cans/list/CanList.helpers.js index d37d8d2dbf..74c1b7534a 100644 --- a/frontend/src/pages/cans/list/CanList.helpers.js +++ b/frontend/src/pages/cans/list/CanList.helpers.js @@ -89,7 +89,9 @@ const applyAdditionalFilters = (cans, filters) => { } if (filters.portfolio && filters.portfolio.length > 0) { filteredCANs = filteredCANs.filter((can) => - filters.portfolio?.some((portfolio) => portfolio.title.toUpperCase() === can.portfolio.name.toUpperCase()) + filters.portfolio?.some( + (portfolio) => portfolio.title == `${can.portfolio.name} (${can.portfolio.abbreviation})` + ) ); } // TODO: Add other filters here @@ -113,7 +115,7 @@ export const getPortfolioOptions = (cans) => { return []; } const portfolios = cans.reduce((acc, can) => { - acc.add(can.portfolio.name); + acc.add(`${can.portfolio.name} (${can.portfolio.abbreviation})`); return acc; }, new Set()); diff --git a/frontend/src/pages/cans/list/CanList.helpers.test.js b/frontend/src/pages/cans/list/CanList.helpers.test.js index 727ef3e415..d29df5c996 100644 --- a/frontend/src/pages/cans/list/CanList.helpers.test.js +++ b/frontend/src/pages/cans/list/CanList.helpers.test.js @@ -139,21 +139,21 @@ describe("Portfolio filtering and options", () => { { id: 1, obligate_by: "2023-12-31", - portfolio: { division_id: 1, abbreviation: "ABC" }, + portfolio: { division_id: 1, name: "Portfolio A", abbreviation: "ABC" }, budget_line_items: [{ team_members: [{ id: 1 }] }], active_period: 1 }, { id: 2, obligate_by: "2023-11-30", - portfolio: { division_id: 2, abbreviation: "XYZ" }, + portfolio: { division_id: 2, name: "Portfolio X", abbreviation: "XYZ" }, budget_line_items: [], active_period: 2 }, { id: 3, obligate_by: "2023-10-31", - portfolio: { division_id: 1, abbreviation: "ABC" }, + portfolio: { division_id: 1, name: "Portfolio A", abbreviation: "ABC" }, budget_line_items: [{ team_members: [{ id: 2 }] }], active_period: 1 } @@ -161,18 +161,20 @@ describe("Portfolio filtering and options", () => { it("should filter CANs by portfolio", () => { const filtersWithPortfolio = { - portfolio: [{ id: 1, title: "ABC" }] + portfolio: [{ id: 1, title: "Portfolio A (ABC)" }] }; const result = sortAndFilterCANs(mockCANsWithPortfolios, false, mockUser, filtersWithPortfolio); expect(result.length).toBe(2); - expect(result.every((can) => can.portfolio.abbreviation === "ABC")).toBe(true); + expect( + result.every((can) => can.portfolio.name === "Portfolio A" && can.portfolio.abbreviation === "ABC") + ).toBe(true); }); it("should return unique portfolio options", () => { const portfolioOptions = getPortfolioOptions(mockCANsWithPortfolios); expect(portfolioOptions).toEqual([ - { id: 0, title: "ABC" }, - { id: 1, title: "XYZ" } + { id: 0, title: "Portfolio A (ABC)" }, + { id: 1, title: "Portfolio X (XYZ)" } ]); }); @@ -184,8 +186,8 @@ describe("Portfolio filtering and options", () => { it("should handle multiple portfolios in filter", () => { const filtersWithMultiplePortfolios = { portfolio: [ - { id: 1, title: "ABC" }, - { id: 2, title: "XYZ" } + { id: 1, title: "Portfolio A (ABC)" }, + { id: 2, title: "Portfolio X (XYZ)" } ] }; const result = sortAndFilterCANs(mockCANsWithPortfolios, false, mockUser, filtersWithMultiplePortfolios);