From 6c8220e14048205c68c34ed9c5021adf68fc1414 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 27 Jun 2022 14:29:42 -0400 Subject: [PATCH] Add tests to cover rearragement - Fixup styling - Move isSelected out of `Options` to prevent reflow and into seperate model function Signed-off-by: Sebastian Malton --- .../namespace-select-filter.test.tsx.snap | 34 ++++---- .../namespace-select-filter-model.tsx | 77 ++++++++++--------- .../+namespaces/namespace-select-filter.scss | 4 + .../namespace-select-filter.test.tsx | 18 ++++- .../+namespaces/namespace-select-filter.tsx | 7 +- 5 files changed, 81 insertions(+), 59 deletions(-) diff --git a/src/renderer/components/+namespaces/__snapshots__/namespace-select-filter.test.tsx.snap b/src/renderer/components/+namespaces/__snapshots__/namespace-select-filter.test.tsx.snap index 8b6d070b2c..5d147fea18 100644 --- a/src/renderer/components/+namespaces/__snapshots__/namespace-select-filter.test.tsx.snap +++ b/src/renderer/components/+namespaces/__snapshots__/namespace-select-filter.test.tsx.snap @@ -191,7 +191,7 @@ exports[` when clicked renders 1`] = `
@@ -226,7 +226,7 @@ exports[` when clicked renders 1`] = `
@@ -261,7 +261,7 @@ exports[` when clicked renders 1`] = `
@@ -296,7 +296,7 @@ exports[` when clicked renders 1`] = `
@@ -331,7 +331,7 @@ exports[` when clicked renders 1`] = `
@@ -366,7 +366,7 @@ exports[` when clicked renders 1`] = `
@@ -401,7 +401,7 @@ exports[` when clicked renders 1`] = `
@@ -436,7 +436,7 @@ exports[` when clicked renders 1`] = `
@@ -471,7 +471,7 @@ exports[` when clicked renders 1`] = `
@@ -506,7 +506,7 @@ exports[` when clicked renders 1`] = `
@@ -541,7 +541,7 @@ exports[` when clicked renders 1`] = `
@@ -576,7 +576,7 @@ exports[` when clicked renders 1`] = `
@@ -611,7 +611,7 @@ exports[` when clicked renders 1`] = `
@@ -841,7 +841,7 @@ exports[` when clicked when 'test-2' is clicked when cl
@@ -1254,7 +1254,7 @@ exports[` when clicked when 'test-2' is clicked when cl `; -exports[` when clicked when 'test-2' is clicked when clicked again when 'test-1' is clicked when clicked again, then holding down multi select key, and then clicking 'test-3' when 'test-3' is clicked renders 1`] = ` +exports[` when clicked when 'test-2' is clicked when clicked again when 'test-1' is clicked when clicked again, then holding down multi select key when 'test-3' is clicked renders 1`] = `
when clicked when 'test-2' is clicked when cl
@@ -1418,7 +1418,7 @@ exports[` when clicked when 'test-2' is clicked when cl
diff --git a/src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.tsx b/src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.tsx index 5bdfa3b479..fab9350a20 100644 --- a/src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.tsx +++ b/src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.tsx @@ -22,39 +22,20 @@ export const selectAllNamespaces = Symbol("all-namespaces-selected"); export type SelectAllNamespaces = typeof selectAllNamespaces; export type NamespaceSelectFilterOption = SelectOption; -export function formatOptionLabel({ value, isSelected }: NamespaceSelectFilterOption) { - if (value === selectAllNamespaces) { - return <>All Namespaces; - } - - return ( -
- - {value} - {isSelected && ( - - )} -
- ); -} - export interface NamespaceSelectFilterModel { readonly options: IComputedValue; readonly menu: { open: () => void; close: () => void; - readonly isOpen: boolean; + readonly isOpen: IComputedValue; }; onChange: (newValue: MultiValue, actionMeta: ActionMeta) => void; onClick: () => void; onKeyDown: React.KeyboardEventHandler; onKeyUp: React.KeyboardEventHandler; reset: () => void; + isOptionSelected: (option: NamespaceSelectFilterOption) => boolean; + formatOptionLabel: (option: NamespaceSelectFilterOption) => JSX.Element; } enum SelectMenuState { @@ -81,7 +62,7 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names const optionsSortingSelected = observable.set(selectedNames.get()); const options = computed((): readonly NamespaceSelectFilterOption[] => { const baseOptions = namespaceStore.items.map(ns => ns.getName()); - const namespaces = selectedNames.get(); + // const namespaces = selectedNames.get(); baseOptions.sort(( (left, right) => @@ -94,17 +75,24 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names value: selectAllNamespaces, label: "All Namespaces", id: "all-namespaces", - isSelected: false, + // isSelected: false, }, ...baseOptions.map(namespace => ({ value: namespace, label: namespace, id: namespace, - isSelected: namespaces.has(namespace), + // isSelected: namespaces.has(namespace), })), ]; }); const menuIsOpen = computed(() => menuState.get() === SelectMenuState.Open); + const isOptionSelected: NamespaceSelectFilterModel["isOptionSelected"] = (option) => { + if (option.value === selectAllNamespaces) { + return false; + } + + return selectedNames.get().has(option.value); + }; const model: NamespaceSelectFilterModel = { options, @@ -115,9 +103,7 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names open: action(() => { menuState.set(SelectMenuState.Open); }), - get isOpen() { - return menuIsOpen.get(); - }, + isOpen: menuIsOpen, }, onChange: (_, action) => { switch (action.action) { @@ -125,19 +111,13 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names namespaceStore.selectAll(); break; case "deselect-option": - if (typeof action.option === "string") { - didToggle = true; - namespaceStore.toggleSingle(action.option); - } - break; case "select-option": - if (action.option?.value === selectAllNamespaces) { - didToggle = true; - namespaceStore.selectAll(); - } else if (action.option) { + if (action.option) { didToggle = true; - if (isMultiSelection) { + if (action.option.value === selectAllNamespaces) { + namespaceStore.selectAll(); + } else if (isMultiSelection) { namespaceStore.toggleSingle(action.option.value); } else { namespaceStore.selectSingle(action.option.value); @@ -171,6 +151,27 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names isMultiSelection = false; model.menu.close(); }), + isOptionSelected, + formatOptionLabel: (option) => { + if (option.value === selectAllNamespaces) { + return <>All Namespaces; + } + + return ( +
+ + {option.value} + {isOptionSelected(option) && ( + + )} +
+ ); + }, }; return model; diff --git a/src/renderer/components/+namespaces/namespace-select-filter.scss b/src/renderer/components/+namespaces/namespace-select-filter.scss index 536b9c794d..869d647557 100644 --- a/src/renderer/components/+namespaces/namespace-select-filter.scss +++ b/src/renderer/components/+namespaces/namespace-select-filter.scss @@ -64,6 +64,10 @@ word-break: break-all; padding: 4px 8px; border-radius: 3px; + + &--is-selected:not(&--is-focused) { + background: transparent; + } } } diff --git a/src/renderer/components/+namespaces/namespace-select-filter.test.tsx b/src/renderer/components/+namespaces/namespace-select-filter.test.tsx index 951e7ce10d..a81e32e263 100644 --- a/src/renderer/components/+namespaces/namespace-select-filter.test.tsx +++ b/src/renderer/components/+namespaces/namespace-select-filter.test.tsx @@ -131,7 +131,7 @@ describe("", () => { expect(result.baseElement.querySelector("#react-select-namespace-select-filter-listbox")).toBeNull(); }); - describe("when clicked again, then holding down multi select key, and then clicking 'test-3'", () => { + describe("when clicked again, then holding down multi select key", () => { beforeEach(() => { const filter = result.getByTestId("namespace-select-filter"); @@ -160,6 +160,22 @@ describe("", () => { expect(result.queryByTestId("namespace-select-filter-option-kube-system-selected")).toBeNull(); }); + describe("when 'test-13' is clicked", () => { + beforeEach(() => { + result.getByText("test-13").click(); + }); + + it("has all of 'test-1', 'test-3', and 'test-13' selected in the store", () => { + expect(new Set(namespaceStore.contextNamespaces)).toEqual(new Set(["test-1", "test-3", "test-13"])); + }); + + it("'test-13' is not sorted to the top of the list", () => { + const topLevelElement = result.getByText("test-13").parentElement?.parentElement as HTMLElement; + + expect(topLevelElement.nextSibling).toBe(null); + }); + }); + describe("when releasing multi select key", () => { beforeEach(() => { const filter = result.getByTestId("namespace-select-filter"); diff --git a/src/renderer/components/+namespaces/namespace-select-filter.tsx b/src/renderer/components/+namespaces/namespace-select-filter.tsx index a5deafc60b..de1c2a8cd6 100644 --- a/src/renderer/components/+namespaces/namespace-select-filter.tsx +++ b/src/renderer/components/+namespaces/namespace-select-filter.tsx @@ -13,7 +13,6 @@ import type { NamespaceStore } from "./store"; import { Select } from "../select"; import { withInjectables } from "@ogre-tools/injectable-react"; import type { NamespaceSelectFilterModel, NamespaceSelectFilterOption, SelectAllNamespaces } from "./namespace-select-filter-model/namespace-select-filter-model"; -import { formatOptionLabel } from "./namespace-select-filter-model/namespace-select-filter-model"; import namespaceSelectFilterModelInjectable from "./namespace-select-filter-model/namespace-select-filter-model.injectable"; import namespaceStoreInjectable from "./store.injectable"; @@ -37,16 +36,18 @@ const NonInjectedNamespaceSelectFilter = observer(({ model, id }: Dependencies & id={id} isMulti={true} isClearable={false} - menuIsOpen={model.menu.isOpen} + menuIsOpen={model.menu.isOpen.get()} components={{ Placeholder }} closeMenuOnSelect={false} controlShouldRenderValue={false} onChange={model.onChange} onBlur={model.reset} - formatOptionLabel={formatOptionLabel} + formatOptionLabel={model.formatOptionLabel} options={model.options.get()} className="NamespaceSelect NamespaceSelectFilter" menuClass="NamespaceSelectFilterMenu" + isOptionSelected={model.isOptionSelected} + hideSelectedOptions={false} />
));