From 58cd551a304a03e42637e6858982f8c5dfd9f598 Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Fri, 10 Nov 2023 13:16:53 +0100 Subject: [PATCH] Refactor user selection state handling using hook This commit introduces `useUserSelectionState` compositional hook. it centralizes and allows reusing the logic for tracking and mutation user selection state across the application. The change aims to increase code reusability, simplify the code, improve testability, and adhere to the single responsibility principle. It makes the code more reliable against race conditions and removes the need for tracking deep changes. Other supporting changes: - Introduce `CardStateIndicator` component for displaying the card state indicator icon, improving the testability and separation of concerns. - Refactor `SelectionTypeHandler` to use functional code with more clear interfaces to simplify the code. It reduces complexity, increases maintainability and increase readability by explicitly separating mutating functions. - Add new unit tests and extend improving ones to cover the new logic introduced in this commit. Remove the need to mount a wrapper component to simplify and optimize some tests, using parameter injection to inject dependencies intead. --- .../Events/IEventSubscriptionCollection.ts | 2 +- .../bootstrapping/DependencyProvider.ts | 9 + .../Menu/Selector/SelectionTypeHandler.ts | 92 +++--- .../Scripts/Menu/Selector/TheSelector.vue | 58 ++-- .../Scripts/View/Cards/CardListItem.vue | 48 +-- .../View/Cards/CardSelectionIndicator.vue | 55 ++++ .../View/Tree/NodeContent/RevertToggle.vue | 63 ++-- .../Scripts/View/Tree/ScriptsTree.vue | 6 +- .../UseCollectionSelectionStateUpdater.ts | 20 +- .../UseSelectedScriptNodeIds.ts | 45 +-- .../Shared/Hooks/UseUserSelectionState.ts | 48 +++ src/presentation/injectionSymbols.ts | 2 + .../bootstrapping/DependencyProvider.spec.ts | 1 + .../Selector/SelectionTypeHandler.spec.ts | 45 +-- ...UseCollectionSelectionStateUpdater.spec.ts | 93 +++--- .../UseSelectedScriptNodeIds.spec.ts | 194 ++----------- .../UseTreeViewFilterEvent.spec.ts | 2 +- .../Hooks/UseUserSelectionState.spec.ts | 273 ++++++++++++++++++ .../Stubs/EventSubscriptionCollectionStub.ts | 1 + .../shared/Stubs/UseCollectionStateStub.ts | 50 +++- .../shared/Stubs/UseUserSelectionStateStub.ts | 49 ++++ tests/unit/shared/Stubs/UserSelectionStub.ts | 14 + 22 files changed, 700 insertions(+), 470 deletions(-) create mode 100644 src/presentation/components/Scripts/View/Cards/CardSelectionIndicator.vue create mode 100644 src/presentation/components/Shared/Hooks/UseUserSelectionState.ts create mode 100644 tests/unit/presentation/components/Shared/Hooks/UseUserSelectionState.spec.ts create mode 100644 tests/unit/shared/Stubs/UseUserSelectionStateStub.ts diff --git a/src/infrastructure/Events/IEventSubscriptionCollection.ts b/src/infrastructure/Events/IEventSubscriptionCollection.ts index a8b0c9ab..1836cc4e 100644 --- a/src/infrastructure/Events/IEventSubscriptionCollection.ts +++ b/src/infrastructure/Events/IEventSubscriptionCollection.ts @@ -5,5 +5,5 @@ export interface IEventSubscriptionCollection { register(subscriptions: IEventSubscription[]): void; unsubscribeAll(): void; - unsubscribeAllAndRegister(subscriptions: IEventSubscription[]); + unsubscribeAllAndRegister(subscriptions: IEventSubscription[]): void; } diff --git a/src/presentation/bootstrapping/DependencyProvider.ts b/src/presentation/bootstrapping/DependencyProvider.ts index 09f67d0b..b2b4d47f 100644 --- a/src/presentation/bootstrapping/DependencyProvider.ts +++ b/src/presentation/bootstrapping/DependencyProvider.ts @@ -11,6 +11,7 @@ import { TransientKey, injectKey, } from '@/presentation/injectionSymbols'; import { PropertyKeys } from '@/TypeHelpers'; +import { useUserSelectionState } from '@/presentation/components/Shared/Hooks/UseUserSelectionState'; export function provideDependencies( context: IApplicationContext, @@ -48,6 +49,14 @@ export function provideDependencies( return useCurrentCode(state, events); }, ), + useUserSelectionState: (di) => di.provide( + InjectionKeys.useUserSelectionState, + () => { + const events = di.injectKey((keys) => keys.useAutoUnsubscribedEvents); + const state = di.injectKey((keys) => keys.useCollectionState); + return useUserSelectionState(state, events); + }, + ), }; registerAll(Object.values(resolvers), api); } diff --git a/src/presentation/components/Scripts/Menu/Selector/SelectionTypeHandler.ts b/src/presentation/components/Scripts/Menu/Selector/SelectionTypeHandler.ts index c753619c..b0acaaa2 100644 --- a/src/presentation/components/Scripts/Menu/Selector/SelectionTypeHandler.ts +++ b/src/presentation/components/Scripts/Menu/Selector/SelectionTypeHandler.ts @@ -2,7 +2,8 @@ import { IScript } from '@/domain/IScript'; import { SelectedScript } from '@/application/Context/State/Selection/SelectedScript'; import { RecommendationLevel } from '@/domain/RecommendationLevel'; import { scrambledEqual } from '@/application/Common/Array'; -import { ICategoryCollectionState, IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState'; +import { IReadOnlyUserSelection, IUserSelection } from '@/application/Context/State/Selection/IUserSelection'; +import { ICategoryCollection } from '@/domain/ICategoryCollection'; export enum SelectionType { Standard, @@ -12,66 +13,79 @@ export enum SelectionType { Custom, } -export class SelectionTypeHandler { - constructor(private readonly state: ICategoryCollectionState) { - if (!state) { throw new Error('missing state'); } - } - - public selectType(type: SelectionType) { - if (type === SelectionType.Custom) { - throw new Error('cannot select custom type'); - } - const selector = selectors.get(type); - selector.select(this.state); - } - - public getCurrentSelectionType(): SelectionType { - for (const [type, selector] of selectors.entries()) { - if (selector.isSelected(this.state)) { - return type; - } - } - return SelectionType.Custom; +export function setCurrentSelectionType(type: SelectionType, context: SelectionMutationContext) { + if (type === SelectionType.Custom) { + throw new Error('cannot select custom type'); } + const selector = selectors.get(type); + selector.select(context); } -interface ISingleTypeHandler { - isSelected: (state: IReadOnlyCategoryCollectionState) => boolean; - select: (state: ICategoryCollectionState) => void; +export function getCurrentSelectionType(context: SelectionCheckContext): SelectionType { + for (const [type, selector] of selectors.entries()) { + if (selector.isSelected(context)) { + return type; + } + } + return SelectionType.Custom; } -const selectors = new Map([ +export interface SelectionCheckContext { + readonly selection: IReadOnlyUserSelection; + readonly collection: ICategoryCollection; +} + +export interface SelectionMutationContext { + readonly selection: IUserSelection, + readonly collection: ICategoryCollection, +} + +interface SelectionTypeHandler { + isSelected: (context: SelectionCheckContext) => boolean; + select: (context: SelectionMutationContext) => void; +} + +const selectors = new Map([ [SelectionType.None, { - select: (state) => state.selection.deselectAll(), - isSelected: (state) => state.selection.selectedScripts.length === 0, + select: ({ selection }) => selection.deselectAll(), + isSelected: ({ selection }) => selection.selectedScripts.length === 0, }], [SelectionType.Standard, getRecommendationLevelSelector(RecommendationLevel.Standard)], [SelectionType.Strict, getRecommendationLevelSelector(RecommendationLevel.Strict)], [SelectionType.All, { - select: (state) => state.selection.selectAll(), - isSelected: (state) => state.selection.selectedScripts.length === state.collection.totalScripts, + select: ({ selection }) => selection.selectAll(), + isSelected: ( + { selection, collection }, + ) => selection.selectedScripts.length === collection.totalScripts, }], ]); -function getRecommendationLevelSelector(level: RecommendationLevel): ISingleTypeHandler { +function getRecommendationLevelSelector( + level: RecommendationLevel, +): SelectionTypeHandler { return { - select: (state) => selectOnly(level, state), - isSelected: (state) => hasAllSelectedLevelOf(level, state), + select: (context) => selectOnly(level, context), + isSelected: (context) => hasAllSelectedLevelOf(level, context), }; } function hasAllSelectedLevelOf( level: RecommendationLevel, - state: IReadOnlyCategoryCollectionState, -) { - const scripts = state.collection.getScriptsByLevel(level); - const { selectedScripts } = state.selection; + context: SelectionCheckContext, +): boolean { + const { collection, selection } = context; + const scripts = collection.getScriptsByLevel(level); + const { selectedScripts } = selection; return areAllSelected(scripts, selectedScripts); } -function selectOnly(level: RecommendationLevel, state: ICategoryCollectionState) { - const scripts = state.collection.getScriptsByLevel(level); - state.selection.selectOnly(scripts); +function selectOnly( + level: RecommendationLevel, + context: SelectionMutationContext, +): void { + const { collection, selection } = context; + const scripts = collection.getScriptsByLevel(level); + selection.selectOnly(scripts); } function areAllSelected( diff --git a/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue b/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue index e493f54c..e207eada 100644 --- a/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue +++ b/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue @@ -65,14 +65,15 @@ diff --git a/src/presentation/components/Scripts/View/Tree/NodeContent/RevertToggle.vue b/src/presentation/components/Scripts/View/Tree/NodeContent/RevertToggle.vue index 114cf32c..aaefe83f 100644 --- a/src/presentation/components/Scripts/View/Tree/NodeContent/RevertToggle.vue +++ b/src/presentation/components/Scripts/View/Tree/NodeContent/RevertToggle.vue @@ -1,6 +1,6 @@