Refactor user selection state handling using hook

This commit introduces `useUserSelection`, a new compositional hook designed to centralize and reuse the logic for tracking and mutating 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 error-prone due to race conditions and independent of Vue's deep tracking. Other supporting changes: - A new `CardStateIndicator` component is introduced for displaying the card state indicator icon, enhancing the testability and separation of concerns. - `SelectionTypeHandler` is refactored to use functional code with more clear interfaces to simplify the codebase. This should reduce complexity, increase maintainability and increase readability by explicitly separating mutating functions. - Unit tests have been added to cover the new logic introduced in this refactor.
undergroundwires · Nov 9, 2023 · 93736d9 · 93736d9
1 parent 7770a9b
commit 93736d9
Show file tree

Hide file tree

Showing 22 changed files with 701 additions and 465 deletions.
diff --git 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
@@ -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
@@ -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'); }
+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);
+}
 
-  public selectType(type: SelectionType) {
-    if (type === SelectionType.Custom) {
-      throw new Error('cannot select custom type');
+export function getCurrentSelectionType(context: SelectionCheckContext): SelectionType {
+  for (const [type, selector] of selectors.entries()) {
+    if (selector.isSelected(context)) {
+      return type;
     }
-    const selector = selectors.get(type);
-    selector.select(this.state);
   }
+  return SelectionType.Custom;
+}
 
-  public getCurrentSelectionType(): SelectionType {
-    for (const [type, selector] of selectors.entries()) {
-      if (selector.isSelected(this.state)) {
-        return type;
-      }
-    }
-    return SelectionType.Custom;
-  }
+export interface SelectionCheckContext {
+  readonly selection: IReadOnlyUserSelection;
+  readonly collection: ICategoryCollection;
 }
 
-interface ISingleTypeHandler {
-  isSelected: (state: IReadOnlyCategoryCollectionState) => boolean;
-  select: (state: ICategoryCollectionState) => void;
+export interface SelectionMutationContext {
+  readonly selection: IUserSelection,
+  readonly collection: ICategoryCollection,
 }
 
-const selectors = new Map<SelectionType, ISingleTypeHandler>([
+interface SelectionTypeHandler {
+  isSelected: (context: SelectionCheckContext) => boolean;
+  select: (context: SelectionMutationContext) => void;
+}
+
+const selectors = new Map<SelectionType, SelectionTypeHandler>([
   [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
@@ -65,14 +65,15 @@
 </template>
 
 <script lang="ts">
-import { defineComponent, ref } from 'vue';
+import {
+  defineComponent, computed,
+} from 'vue';
 import { injectKey } from '@/presentation/injectionSymbols';
 import TooltipWrapper from '@/presentation/components/Shared/TooltipWrapper.vue';
-import { ICategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
-import { IEventSubscription } from '@/infrastructure/Events/IEventSource';
+import { ICategoryCollection } from '@/domain/ICategoryCollection';
 import MenuOptionList from '../MenuOptionList.vue';
 import MenuOptionListItem from '../MenuOptionListItem.vue';
-import { SelectionType, SelectionTypeHandler } from './SelectionTypeHandler';
+import { SelectionType, setCurrentSelectionType, getCurrentSelectionType } from './SelectionTypeHandler';
 
 export default defineComponent({
   components: {
@@ -81,43 +82,38 @@ export default defineComponent({
     TooltipWrapper,
   },
   setup() {
-    const { modifyCurrentState, onStateChange } = injectKey((keys) => keys.useCollectionState);
-    const { events } = injectKey((keys) => keys.useAutoUnsubscribedEvents);
+    const {
+      currentSelection, modifyCurrentSelection,
+    } = injectKey((keys) => keys.useUserSelectionState);
+    const { currentState } = injectKey((keys) => keys.useCollectionState);
 
-    const currentSelection = ref(SelectionType.None);
+    const currentCollection = computed<ICategoryCollection>(() => currentState.value.collection);
 
-    let selectionTypeHandler: SelectionTypeHandler;
-
-    onStateChange(() => {
-      modifyCurrentState((state) => {
-        selectionTypeHandler = new SelectionTypeHandler(state);
-        updateSelections();
-        events.unsubscribeAllAndRegister([
-          subscribeAndUpdateSelections(state),
-        ]);
-      });
-    }, { immediate: true });
-
-    function subscribeAndUpdateSelections(
-      state: ICategoryCollectionState,
-    ): IEventSubscription {
-      return state.selection.changed.on(() => updateSelections());
-    }
+    const currentSelectionType = computed<SelectionType>({
+      get: () => getCurrentSelectionType({
+        selection: currentSelection.value,
+        collection: currentCollection.value,
+      }),
+      set: (type: SelectionType) => {
+        selectType(type);
+      },
+    });
 
     function selectType(type: SelectionType) {
-      if (currentSelection.value === type) {
+      if (currentSelectionType.value === type) {
         return;
       }
-      selectionTypeHandler.selectType(type);
-    }
-
-    function updateSelections() {
-      currentSelection.value = selectionTypeHandler.getCurrentSelectionType();
+      modifyCurrentSelection((mutableSelection) => {
+        setCurrentSelectionType(type, {
+          selection: mutableSelection,
+          collection: currentCollection.value,
+        });
+      });
     }
 
     return {
       SelectionType,
-      currentSelection,
+      currentSelection: currentSelectionType,
       selectType,
     };
   },

diff --git a/src/presentation/components/Scripts/View/Cards/CardListItem.vue b/src/presentation/components/Scripts/View/Cards/CardListItem.vue
@@ -22,16 +22,10 @@
         :icon="isExpanded ? 'folder-open' : 'folder'"
       />
       <!-- Indeterminate and full states -->
-      <div class="card__inner__state-icons">
-        <AppIcon
-          icon="battery-half"
-          v-if="isAnyChildSelected && !areAllChildrenSelected"
-        />
-        <AppIcon
-          icon="battery-full"
-          v-if="areAllChildrenSelected"
-        />
-      </div>
+      <CardSelectionIndicator
+        class="card__inner__selection_indicator"
+        :categoryId="categoryId"
+      />
     </div>
     <div class="card__expander" v-on:click.stop>
       <div class="card__expander__content">
@@ -49,17 +43,19 @@
 
 <script lang="ts">
 import {
-  defineComponent, ref, watch, computed, shallowRef,
+  defineComponent, computed, shallowRef,
 } from 'vue';
 import AppIcon from '@/presentation/components/Shared/Icon/AppIcon.vue';
 import { injectKey } from '@/presentation/injectionSymbols';
 import ScriptsTree from '@/presentation/components/Scripts/View/Tree/ScriptsTree.vue';
 import { sleep } from '@/infrastructure/Threading/AsyncSleep';
+import CardSelectionIndicator from './CardSelectionIndicator.vue';
 
 export default defineComponent({
   components: {
     ScriptsTree,
     AppIcon,
+    CardSelectionIndicator,
   },
   props: {
     categoryId: {
@@ -77,8 +73,7 @@ export default defineComponent({
     /* eslint-enable @typescript-eslint/no-unused-vars */
   },
   setup(props, { emit }) {
-    const { onStateChange, currentState } = injectKey((keys) => keys.useCollectionState);
-    const { events } = injectKey((keys) => keys.useAutoUnsubscribedEvents);
+    const { currentState } = injectKey((keys) => keys.useCollectionState);
 
     const isExpanded = computed({
       get: () => {
@@ -92,8 +87,6 @@ export default defineComponent({
       },
     });
 
-    const isAnyChildSelected = ref(false);
-    const areAllChildrenSelected = ref(false);
     const cardElement = shallowRef<HTMLElement>();
 
     const cardTitle = computed<string | undefined>(() => {
@@ -108,37 +101,14 @@ export default defineComponent({
       isExpanded.value = false;
     }
 
-    onStateChange((state) => {
-      events.unsubscribeAllAndRegister([
-        state.selection.changed.on(
-          () => updateSelectionIndicators(props.categoryId),
-        ),
-      ]);
-      updateSelectionIndicators(props.categoryId);
-    }, { immediate: true });
-
-    watch(
-      () => props.categoryId,
-      (categoryId) => updateSelectionIndicators(categoryId),
-    );
-
     async function scrollToCard() {
       await sleep(400); // wait a bit to allow GUI to render the expanded card
       cardElement.value.scrollIntoView({ behavior: 'smooth' });
     }
 
-    function updateSelectionIndicators(categoryId: number) {
-      const category = currentState.value.collection.findCategory(categoryId);
-      const { selection } = currentState.value;
-      isAnyChildSelected.value = category ? selection.isAnySelected(category) : false;
-      areAllChildrenSelected.value = category ? selection.areAllSelected(category) : false;
-    }
-
     return {
       cardTitle,
       isExpanded,
-      isAnyChildSelected,
-      areAllChildrenSelected,
       cardElement,
       collapse,
     };
@@ -192,7 +162,7 @@ $card-horizontal-gap    : $card-gap;
       flex: 1;
       justify-content: center;
     }
-    &__state-icons {
+    &__selection_indicator {
       height: $card-inner-padding;
       margin-right: -$card-inner-padding;
       padding-right: 10px;