From ec83de36009b506ddfee3110847dad4effe7ef42 Mon Sep 17 00:00:00 2001
From: Konstantin Schaper <konstantin.schaper@cloudogu.com>
Date: Thu, 3 Nov 2022 19:17:39 +0100
Subject: [PATCH] Fix modal de-registration firing twice (#2145)

Global Modals as components, and as a result their registration hooks, have a lifecycle that spans the whole application. Because of this, they never get unmounted. The registration hook will simply be re-rendered with an updated "active" flag. Because the hook did not reset to its initial state when switching from "active" to "deactive", it decremented the modal count twice. Once in the cleanup of the previous hook render and once in the else block ("inactive").
---
 gradle/changelog/fix_modal_registration.yaml  |  2 ++
 .../src/modals/useRegisterModal.test.tsx      | 19 +++++++++++++++++--
 .../src/modals/useRegisterModal.ts            |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 gradle/changelog/fix_modal_registration.yaml

diff --git a/gradle/changelog/fix_modal_registration.yaml b/gradle/changelog/fix_modal_registration.yaml
new file mode 100644
index 0000000000..f32914cc9e
--- /dev/null
+++ b/gradle/changelog/fix_modal_registration.yaml
@@ -0,0 +1,2 @@
+- type: fixed
+  description: Keyboard shortcuts should be inactive when modals are open ([#2145](https://github.com/scm-manager/scm-manager/pull/2145))
diff --git a/scm-ui/ui-components/src/modals/useRegisterModal.test.tsx b/scm-ui/ui-components/src/modals/useRegisterModal.test.tsx
index 3fa6207cb5..22f8f19319 100644
--- a/scm-ui/ui-components/src/modals/useRegisterModal.test.tsx
+++ b/scm-ui/ui-components/src/modals/useRegisterModal.test.tsx
@@ -72,7 +72,7 @@ describe("useRegisterModal", () => {
     expect(increment).not.toHaveBeenCalled();
   });
 
-  it("should decrement on active de-registration", () => {
+  it("should decrement once on active de-registration", () => {
     const increment = jest.fn();
     const decrement = jest.fn();
 
@@ -82,7 +82,22 @@ describe("useRegisterModal", () => {
 
     result.unmount();
 
-    expect(decrement).toHaveBeenCalled();
+    expect(decrement).toHaveBeenCalledTimes(1);
     expect(increment).not.toHaveBeenCalled();
   });
+
+  it("should decrement once when switching from active to inactive", () => {
+    const increment = jest.fn();
+    const decrement = jest.fn();
+
+    const result = renderHook(({ active }: { active: boolean }) => useRegisterModal(active), {
+      wrapper: createWrapper({ value: 0, increment, decrement }),
+      initialProps: { active: true },
+    });
+
+    result.rerender({ active: false });
+
+    expect(decrement).toHaveBeenCalledTimes(1);
+    expect(increment).toHaveBeenCalledTimes(1);
+  });
 });
diff --git a/scm-ui/ui-components/src/modals/useRegisterModal.ts b/scm-ui/ui-components/src/modals/useRegisterModal.ts
index f8dbc75067..2d73951b1c 100644
--- a/scm-ui/ui-components/src/modals/useRegisterModal.ts
+++ b/scm-ui/ui-components/src/modals/useRegisterModal.ts
@@ -47,6 +47,7 @@ export default function useRegisterModal(active: boolean, initialValue: boolean
     return () => {
       if (previousActiveState.current) {
         decrement();
+        previousActiveState.current = null;
       }
     };
   }, [active, decrement, increment]);