From 03fedab46318e0a2d9d6ee2a2558b25f73331598 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 14 Jan 2025 14:20:11 -0800 Subject: [PATCH 1/5] FilterMenu: support a "defaultExcludedValues" prop It took a long time to get this to behave intuitively. In short, it took a long to spec it out. How to have it behave when you click one of the values, how to represent the default state, ... The feature as implemented: * FilterMenu has a new prop defaultExcludedValues: string[] * Without defaultExcludedValues set, it operates exactly as it does now * When set, the UI has one new "showDefault" state in addition to existing "showAll" * In "showDefault", Pulls matching one of the excluded values are hidden (these excluded values appear de-selected in the UI) * One click "selects" them and leaves "showDefault" mode * "Show All" overrides the "default exclusions" and shows everything --- frontend/src/filter-menu.tsx | 75 +++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/frontend/src/filter-menu.tsx b/frontend/src/filter-menu.tsx index 3cbf1776..5fc3b8ae 100644 --- a/frontend/src/filter-menu.tsx +++ b/frontend/src/filter-menu.tsx @@ -17,22 +17,29 @@ import { countBy } from "lodash-es"; // Map from value to number of pulls that have that value type ValueGetter = (pull: Pull) => string; +const SHOWALL = "SHOWALL"; + type FilterMenuProps = { urlParam: string; buttonText: string; extractValueFromPull: ValueGetter; + defaultExculdedValues?: string[]; }; export function FilterMenu({ urlParam, buttonText, extractValueFromPull, + defaultExculdedValues, }: FilterMenuProps) { const pulls = useAllOpenPulls(); // Default is empty array that implies show all pulls (no filtering) const [selectedValues, setSelectedValues] = useArrayUrlState(urlParam, []); - // Nothing selected == show everything, otherwise, it'd be empty - const showAll = selectedValues.length === 0; + // Nothing selected == show the default values (everything except the excluded values) + const showDefault = selectedValues.length === 0; + // Show every single value if the magic SHOWALL string is selected + const showAll = notEmpty(defaultExculdedValues) ? selectedValues.includes(SHOWALL) : selectedValues.length === 0; + // List from url may contain values we have no pulls for const urlValues = useConst(() => new Set(selectedValues)); const setPullFilter = useSetFilter(); @@ -41,20 +48,26 @@ export function FilterMenu({ const allValues = useMemo(() => { // All values of open pulls const pullValues = new Set(pulls.map(extractValueFromPull)); - return sortValues([...new Set([...pullValues, ...urlValues])]); + const allValuesSet = new Set([...pullValues, ...urlValues]); + allValuesSet.delete(SHOWALL); + return sortValues([...allValuesSet]); }, [pulls]); + const valueToPullCount = useMemo( () => countBy(pulls, extractValueFromPull), [pulls] ); + const defaultSelectedValues = arrayDiff(allValues, defaultExculdedValues || []); + useEffect(() => { const selectedValuesSet = new Set(selectedValues); setPullFilter( urlParam, - selectedValuesSet.size === 0 + showAll ? null - : (pull) => selectedValuesSet.has(extractValueFromPull(pull)) + : (showDefault ? (pull) => !defaultExculdedValues?.includes(extractValueFromPull(pull)) + : (pull) => selectedValuesSet.has(extractValueFromPull(pull))) ); }, [selectedValues]); @@ -64,22 +77,42 @@ export function FilterMenu({ as={Button} colorScheme="blue" size="sm" - variant={showAll ? "outline" : null} + variant={showDefault ? "outline" : null} > {buttonText} {selectedValues.length ? `(${selectedValues.length})` : ""} - setSelectedValues([])} - isChecked={showAll} - > - Show All - + {notEmpty(defaultExculdedValues) && ( + <> + setSelectedValues([SHOWALL])} + isChecked={showAll} + > + Show All + + setSelectedValues([])} + isChecked={showDefault} + > + Show Default + + ) + } + {empty(defaultExculdedValues) && + setSelectedValues([])} + isChecked={showAll} + > + Show All + + } {allValues.map((value) => ( @@ -98,3 +131,17 @@ function sortValues(values: string[]): string[] { a.localeCompare(b, undefined, { sensitivity: "base" }) ); } + +function arrayDiff(a: T[], b: T[]): T[] { + const ret = a.filter((x) => !b.includes(x)); + console.log(ret); + return ret; +} + +function empty(array: T[] | undefined): boolean { + return !array || array.length === 0; +} + +function notEmpty(array: T[] | undefined): boolean { + return !!array && array.length > 0; +} From 6218bc07341121ae15907bdb31237d6fe7691da0 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Wed, 15 Jan 2025 10:39:28 -0800 Subject: [PATCH 2/5] Repos: Allow hiding some by default from config Provide a way to hide a repo by default from the config by adding an option to the config. We have to pass the repo specs part of the config around more, and transform these `{org}/{repo}` names into just the {name} portion for the filter, but this works for now. --- config.example.js | 2 ++ frontend/src/filter-menu.tsx | 4 ++-- frontend/src/navbar.tsx | 8 +++++++- frontend/src/pull-card/index.tsx | 2 +- frontend/src/pulldasher/pulls-context.tsx | 11 ++++++++++- frontend/src/pulldasher/pulls-state.tsx | 21 ++++++++++++++------- frontend/src/types.ts | 1 + 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/config.example.js b/config.example.js index dc01eefd..e54d33ed 100644 --- a/config.example.js +++ b/config.example.js @@ -75,6 +75,8 @@ module.exports = { name: "owner/otherRepo", requiredStatuses: ["tests", "build", "codeClimate"], ignoredStatuses: ["coverage"], + // Hides pulls on this repo on page load, users can unhide them with the repo filter + hideByDefault: true, }, ], diff --git a/frontend/src/filter-menu.tsx b/frontend/src/filter-menu.tsx index 5fc3b8ae..8755177c 100644 --- a/frontend/src/filter-menu.tsx +++ b/frontend/src/filter-menu.tsx @@ -69,7 +69,7 @@ export function FilterMenu({ : (showDefault ? (pull) => !defaultExculdedValues?.includes(extractValueFromPull(pull)) : (pull) => selectedValuesSet.has(extractValueFromPull(pull))) ); - }, [selectedValues]); + }, [selectedValues, defaultExculdedValues]); return ( @@ -92,7 +92,7 @@ export function FilterMenu({ Show All setSelectedValues([])} isChecked={showDefault} > diff --git a/frontend/src/navbar.tsx b/frontend/src/navbar.tsx index 88944197..825fa8da 100644 --- a/frontend/src/navbar.tsx +++ b/frontend/src/navbar.tsx @@ -4,6 +4,7 @@ import { useFilteredOpenPulls, useAllOpenPulls, useSetFilter, + useRepoSpecs, } from "./pulldasher/pulls-context"; import { Pull } from "./pull"; import { @@ -21,7 +22,7 @@ import { MenuList, Text } from "@chakra-ui/react"; -import { useRef, useEffect, useCallback } from "react"; +import { useRef, useEffect, useCallback, useMemo } from "react"; import { useBoolUrlState } from "./use-url-state"; import { NotificationRequest } from "./notifications"; import { useConnectionState, ConnectionState } from "./backend/socket"; @@ -51,6 +52,10 @@ export function Navbar(props: NavBarProps) { const pulls: Set = useFilteredOpenPulls(); const allOpenPulls: Pull[] = useAllOpenPulls(); const setPullFilter = useSetFilter(); + const repoSpecs = useRepoSpecs(); + const reposToHide = useMemo(() => + repoSpecs.filter((repo) => repo.hideByDefault).map((repo) => repo.name.replace(/.*\//g, "")), + [repoSpecs]); const { toggleColorMode } = useColorMode(); const [showCryo, toggleShowCryo] = useBoolUrlState("cryo", false); const [showExtBlocked, toggleShowExtBlocked] = useBoolUrlState( @@ -174,6 +179,7 @@ export function Navbar(props: NavBarProps) { pull.getRepoName()} /> diff --git a/frontend/src/pull-card/index.tsx b/frontend/src/pull-card/index.tsx index 5a64527e..43d006e7 100644 --- a/frontend/src/pull-card/index.tsx +++ b/frontend/src/pull-card/index.tsx @@ -196,9 +196,9 @@ const formatDate = (dateStr: string | null) => { return dateStr ? formatter.format(new Date(dateStr)) : null; }; -// eslint-disable-next-line @typescript-eslint/no-explicit-any function highlightOnChange( ref: RefObject, +// eslint-disable-next-line @typescript-eslint/no-explicit-any dependencies: Array ) { // Animate a highlight when pull.received_at changes diff --git a/frontend/src/pulldasher/pulls-context.tsx b/frontend/src/pulldasher/pulls-context.tsx index 721baff9..6d49f2a5 100644 --- a/frontend/src/pulldasher/pulls-context.tsx +++ b/frontend/src/pulldasher/pulls-context.tsx @@ -6,6 +6,7 @@ import { } from "./filtered-pulls-state"; import { usePullsState } from "./pulls-state"; import { Pull } from "../pull"; +import { RepoSpec } from "../types"; import { defaultCompare } from "./sort"; interface PullContextProps { @@ -19,6 +20,8 @@ interface PullContextProps { filteredOpenPulls: Set; // Changes the filter function setFilter: FilterFunctionSetter; + // RepoSpecs (.repos) from the config + repoSpecs: RepoSpec[]; } const defaultProps = { @@ -29,6 +32,7 @@ const defaultProps = { // Default implementation is a no-op, just so there's // something there until the provider is used setFilter: (name: string, filter: FilterFunction) => filter, + repoSpecs: [], }; const PullsContext = createContext(defaultProps); @@ -52,12 +56,16 @@ export function useSetFilter(): FilterFunctionSetter { return useContext(PullsContext).setFilter; } +export function useRepoSpecs(): RepoSpec[] { + return useContext(PullsContext).repoSpecs; +} + export const PullsProvider = function ({ children, }: { children: React.ReactNode; }) { - const unfilteredPulls = usePullsState(); + const {pullState: unfilteredPulls, repoSpecs} = usePullsState(); const [filteredPulls, setFilter] = useFilteredPullsState(unfilteredPulls); const openPulls = unfilteredPulls.filter(isOpen); const contextValue = { @@ -66,6 +74,7 @@ export const PullsProvider = function ({ filteredPulls: filteredPulls, allPulls: unfilteredPulls, setFilter, + repoSpecs, }; return ( diff --git a/frontend/src/pulldasher/pulls-state.tsx b/frontend/src/pulldasher/pulls-state.tsx index 6ba8624c..f2f7496a 100644 --- a/frontend/src/pulldasher/pulls-state.tsx +++ b/frontend/src/pulldasher/pulls-state.tsx @@ -4,19 +4,22 @@ import { createPullSocket } from "../backend/pull-socket"; import { PullData, RepoSpec } from "../types"; import { Pull } from "../pull"; -function onPullsChanged(pullsChanged: (pulls: Pull[]) => void) { +function onPullsChanged(pullsChanged: (pulls: Pull[], repoSpecs: RepoSpec[]) => void) { const pulls: Record = {}; - const pullRefresh = () => pullsChanged(Object.values(pulls)); + const pullRefresh = () => pullsChanged(Object.values(pulls), repoSpecs); const throttledPullRefresh: () => void = throttle(pullRefresh, 500); + let repoSpecs: RepoSpec[] = []; - createPullSocket((pullDatas: PullData[], repoSpecs: RepoSpec[]) => { + createPullSocket((pullDatas: PullData[], newRepoSpecs: RepoSpec[]) => { pullDatas.forEach((pullData: PullData) => { pullData.repoSpec = - repoSpecs.find((repo) => repo.name == pullData.repo) || null; + newRepoSpecs.find((repo) => repo.name == pullData.repo) || null; pullData.received_at = new Date(); const pull: Pull = new Pull(pullData); pulls[pull.getKey()] = pull; }); + + repoSpecs = newRepoSpecs || []; throttledPullRefresh(); }); } @@ -25,8 +28,9 @@ function onPullsChanged(pullsChanged: (pulls: Pull[]) => void) { * Note: This is only meant to be used in one component */ let socketInitialized = false; -export function usePullsState(): Pull[] { +export function usePullsState() { const [pullState, setPullsState] = useState([]); + const [repoSpecs, setRepoSpecs] = useState([]); useEffect(() => { if (socketInitialized) { throw new Error( @@ -34,7 +38,10 @@ export function usePullsState(): Pull[] { ); } socketInitialized = true; - onPullsChanged(setPullsState); + onPullsChanged((pulls, repoSpecs) => { + setPullsState(pulls); + setRepoSpecs(repoSpecs); + }); }, []); - return pullState; + return {pullState, repoSpecs}; } diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 6bd74e61..20e1494c 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -32,6 +32,7 @@ export interface SignatureUser { export interface RepoSpec { requiredStatuses?: string[]; ignoredStatuses?: string[]; + hideByDefault?: boolean; name: string; } From aa9cc827ce5f46bcfe91ea9ba4e21b42f1a7aaf1 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Wed, 15 Jan 2025 16:52:28 -0800 Subject: [PATCH 3/5] filter-menu: drop a debug console.log --- frontend/src/filter-menu.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frontend/src/filter-menu.tsx b/frontend/src/filter-menu.tsx index 8755177c..e5a7e707 100644 --- a/frontend/src/filter-menu.tsx +++ b/frontend/src/filter-menu.tsx @@ -133,9 +133,7 @@ function sortValues(values: string[]): string[] { } function arrayDiff(a: T[], b: T[]): T[] { - const ret = a.filter((x) => !b.includes(x)); - console.log(ret); - return ret; + return a.filter((x) => !b.includes(x)); } function empty(array: T[] | undefined): boolean { From 73de4d22712afc18d3e28cf14c2ca38dd0661039 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Wed, 15 Jan 2025 18:03:30 -0800 Subject: [PATCH 4/5] Filter Menu: add "only" option to each menu Instead of "Show All" being a state, it's now just a button. Instead of a click isolating that option when in Show All mode, it will simply uncheck it. This adds an "only" option to every menu item to replace the old functionality. --- frontend/src/filter-menu.tsx | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/frontend/src/filter-menu.tsx b/frontend/src/filter-menu.tsx index e5a7e707..6906ab29 100644 --- a/frontend/src/filter-menu.tsx +++ b/frontend/src/filter-menu.tsx @@ -10,6 +10,7 @@ import { MenuDivider, MenuOptionGroup, MenuItemOption, + chakra, } from "@chakra-ui/react"; import { useEffect, useMemo } from "react"; import { countBy } from "lodash-es"; @@ -87,14 +88,12 @@ export function FilterMenu({ setSelectedValues([SHOWALL])} - isChecked={showAll} > Show All setSelectedValues([])} - isChecked={showDefault} > Show Default @@ -104,7 +103,6 @@ export function FilterMenu({ setSelectedValues([])} - isChecked={showAll} > Show All @@ -112,12 +110,24 @@ export function FilterMenu({ {allValues.map((value) => ( - + {value} ({valueToPullCount[value] || 0}) + { + setSelectedValues([value]); + e.stopPropagation(); + } + }>only ))} From 492605966240416af416b42fddbb05201384fe8b Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Thu, 16 Jan 2025 15:13:27 -0800 Subject: [PATCH 5/5] FilterMenu: show count of all values when showAll is chosen Previously, it would show `{FilterName} (1)` when "Show All" was chosen because it was reporting the number of values in the url. Now, it shows the total count any time the selection deviates from the default. --- frontend/src/filter-menu.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/src/filter-menu.tsx b/frontend/src/filter-menu.tsx index 6906ab29..53d6bf67 100644 --- a/frontend/src/filter-menu.tsx +++ b/frontend/src/filter-menu.tsx @@ -72,6 +72,8 @@ export function FilterMenu({ ); }, [selectedValues, defaultExculdedValues]); + const numberText = showDefault ? "" : (showAll ? allValues.length : selectedValues.length); + return ( - {buttonText} {selectedValues.length ? `(${selectedValues.length})` : ""} + {buttonText} {numberText ? `(${numberText})` : ""} {notEmpty(defaultExculdedValues) && (