From 8ccaec7af6ea3ecfd46bab5c13b90f71d55e32c1 Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Mon, 6 Nov 2023 21:55:43 +0100 Subject: [PATCH] Fix unresponsive copy button on instructions modal This commit fixes the bug where the "Copy" button does not copy when clicked on download instructions modal (on Linux and macOS). This commit also introduces several improvements to the UI components related to copy action and their interaction with the clipboard feature. It adds more tests to avoid regression of the bugs and improves maintainability, testability and adherence to Vue's reactive principles. Changes include: - Fix non-responsive copy button in the download instructions modal by triggering a `click` event in `AppIcon.vue`. - Improve `TheCodeButtons.vue`: - Remove redundant `getCurrentCode` function. - Separate components for each button for better separation of concerns and higher testability. - Use the `gap` property in the flexbox layout, replacing the less explicit sibling combinator approach. - Add `useClipboard` compositional hook for more idiomatic Vue approach to interacting with the clipboard. - Add `useCurrentCode` compositional hook to handle current code state more effectively with unified logic. - Abstract clipboard operations to an interface to isolate responsibilities. - Switch clipboard implementation to the `navigator.clipboard` API, moving away from the deprecated `document.execCommand`. - Move clipboard logic to the presentation layer to conform to separation of concerns and domain-driven design principles. - Improve `IconButton.vue` component to increase reusability with consistent sizing. --- src/infrastructure/Clipboard.ts | 13 -- .../bootstrapping/DependencyProvider.ts | 8 + .../Code/CodeButtons/CodeCopyButton.vue | 33 ++++ .../Code/CodeButtons/CodeRunButton.vue | 59 +++++++ .../Code/CodeButtons/IconButton.vue | 52 +++--- .../Code/CodeButtons/Save/CodeSaveButton.vue | 95 ++++++++++ .../Instructions/CodeInstruction.vue | 10 +- .../Instructions/Data/InstructionsBuilder.ts | 0 .../Data/LinuxInstructionsBuilder.ts | 0 .../Data/MacOsInstructionsBuilder.ts | 0 .../Instructions/InstructionList.vue | 0 .../Instructions/InstructionListData.ts | 0 .../InstructionListDataFactory.ts | 0 .../Code/CodeButtons/TheCodeButtons.vue | 162 ++---------------- .../Hooks/Clipboard/BrowserClipboard.ts | 15 ++ .../Shared/Hooks/Clipboard/Clipboard.ts | 3 + .../Shared/Hooks/Clipboard/UseClipboard.ts | 13 ++ .../components/Shared/Hooks/UseCurrentCode.ts | 32 ++++ .../components/Shared/Icon/AppIcon.vue | 19 +- src/presentation/injectionSymbols.ts | 12 +- .../bootstrapping/DependencyProvider.spec.ts | 2 + .../Code/CodeButtons/CodeCopyButton.spec.ts | 65 +++++++ .../Save/Instructions/CodeInstruction.spec.ts | 96 +++++++++++ .../Data/InstructionsBuilder.spec.ts | 4 +- .../Data/MacOsInstructionsBuilder.spec.ts | 2 +- .../OsSpecificInstructionBuilderTestRunner.ts | 2 +- .../InstructionListDataFactory.spec.ts | 4 +- .../Hooks/Clipboard/BrowserClipboard.spec.ts | 73 ++++++++ .../Hooks/Clipboard/UseClipboard.spec.ts | 67 ++++++++ .../Shared/Hooks/UseCurrentCode.spec.ts | 128 ++++++++++++++ .../components/Shared/Icon/AppIcon.spec.ts | 22 ++- .../unit/shared/Stubs/ApplicationCodeStub.ts | 15 +- .../Stubs/CategoryCollectionStateStub.ts | 7 +- tests/unit/shared/Stubs/ClipboardStub.ts | 14 ++ .../unit/shared/Stubs/CodeChangedEventStub.ts | 26 +++ tests/unit/shared/Stubs/UseClipboardStub.ts | 17 ++ tests/unit/shared/Stubs/UseCurrentCodeStub.ts | 17 ++ 37 files changed, 881 insertions(+), 206 deletions(-) delete mode 100644 src/infrastructure/Clipboard.ts create mode 100644 src/presentation/components/Code/CodeButtons/CodeCopyButton.vue create mode 100644 src/presentation/components/Code/CodeButtons/CodeRunButton.vue create mode 100644 src/presentation/components/Code/CodeButtons/Save/CodeSaveButton.vue rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/CodeInstruction.vue (86%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/InstructionsBuilder.ts (100%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/LinuxInstructionsBuilder.ts (100%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/MacOsInstructionsBuilder.ts (100%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/InstructionList.vue (100%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/InstructionListData.ts (100%) rename src/presentation/components/Code/CodeButtons/{ => Save}/Instructions/InstructionListDataFactory.ts (100%) create mode 100644 src/presentation/components/Shared/Hooks/Clipboard/BrowserClipboard.ts create mode 100644 src/presentation/components/Shared/Hooks/Clipboard/Clipboard.ts create mode 100644 src/presentation/components/Shared/Hooks/Clipboard/UseClipboard.ts create mode 100644 src/presentation/components/Shared/Hooks/UseCurrentCode.ts create mode 100644 tests/unit/presentation/components/Code/CodeButtons/CodeCopyButton.spec.ts create mode 100644 tests/unit/presentation/components/Code/CodeButtons/Save/Instructions/CodeInstruction.spec.ts rename tests/unit/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/InstructionsBuilder.spec.ts (96%) rename tests/unit/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/MacOsInstructionsBuilder.spec.ts (85%) rename tests/unit/presentation/components/Code/CodeButtons/{ => Save}/Instructions/Data/OsSpecificInstructionBuilderTestRunner.ts (93%) rename tests/unit/presentation/components/Code/CodeButtons/{ => Save}/Instructions/InstructionListDataFactory.spec.ts (91%) create mode 100644 tests/unit/presentation/components/Shared/Hooks/Clipboard/BrowserClipboard.spec.ts create mode 100644 tests/unit/presentation/components/Shared/Hooks/Clipboard/UseClipboard.spec.ts create mode 100644 tests/unit/presentation/components/Shared/Hooks/UseCurrentCode.spec.ts create mode 100644 tests/unit/shared/Stubs/ClipboardStub.ts create mode 100644 tests/unit/shared/Stubs/CodeChangedEventStub.ts create mode 100644 tests/unit/shared/Stubs/UseClipboardStub.ts create mode 100644 tests/unit/shared/Stubs/UseCurrentCodeStub.ts diff --git a/src/infrastructure/Clipboard.ts b/src/infrastructure/Clipboard.ts deleted file mode 100644 index ae7f8299..00000000 --- a/src/infrastructure/Clipboard.ts +++ /dev/null @@ -1,13 +0,0 @@ -export class Clipboard { - public static copyText(text: string): void { - const el = document.createElement('textarea'); - el.value = text; - el.setAttribute('readonly', ''); // to avoid focus - el.style.position = 'absolute'; - el.style.left = '-9999px'; - document.body.appendChild(el); - el.select(); - document.execCommand('copy'); - document.body.removeChild(el); - } -} diff --git a/src/presentation/bootstrapping/DependencyProvider.ts b/src/presentation/bootstrapping/DependencyProvider.ts index 01e9a50f..dc392c02 100644 --- a/src/presentation/bootstrapping/DependencyProvider.ts +++ b/src/presentation/bootstrapping/DependencyProvider.ts @@ -5,6 +5,8 @@ import { useAutoUnsubscribedEvents } from '@/presentation/components/Shared/Hook import { IApplicationContext } from '@/application/Context/IApplicationContext'; import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { InjectionKeys } from '@/presentation/injectionSymbols'; +import { useClipboard } from '../components/Shared/Hooks/Clipboard/UseClipboard'; +import { useCurrentCode } from '../components/Shared/Hooks/UseCurrentCode'; export function provideDependencies( context: IApplicationContext, @@ -23,6 +25,12 @@ export function provideDependencies( const { events } = api.inject(InjectionKeys.useAutoUnsubscribedEvents)(); return useCollectionState(context, events); }); + registerTransient(InjectionKeys.useClipboard, () => useClipboard()); + registerTransient(InjectionKeys.useCurrentCode, () => { + const { events } = api.inject(InjectionKeys.useAutoUnsubscribedEvents)(); + const state = api.inject(InjectionKeys.useCollectionState)(); + return useCurrentCode(state, events); + }); } export interface VueDependencyInjectionApi { diff --git a/src/presentation/components/Code/CodeButtons/CodeCopyButton.vue b/src/presentation/components/Code/CodeButtons/CodeCopyButton.vue new file mode 100644 index 00000000..ab20237e --- /dev/null +++ b/src/presentation/components/Code/CodeButtons/CodeCopyButton.vue @@ -0,0 +1,33 @@ + + + diff --git a/src/presentation/components/Code/CodeButtons/CodeRunButton.vue b/src/presentation/components/Code/CodeButtons/CodeRunButton.vue new file mode 100644 index 00000000..97b04a40 --- /dev/null +++ b/src/presentation/components/Code/CodeButtons/CodeRunButton.vue @@ -0,0 +1,59 @@ + + + diff --git a/src/presentation/components/Code/CodeButtons/IconButton.vue b/src/presentation/components/Code/CodeButtons/IconButton.vue index f404be32..49983ce0 100644 --- a/src/presentation/components/Code/CodeButtons/IconButton.vue +++ b/src/presentation/components/Code/CodeButtons/IconButton.vue @@ -1,15 +1,17 @@ diff --git a/src/presentation/components/Code/CodeButtons/Instructions/CodeInstruction.vue b/src/presentation/components/Code/CodeButtons/Save/Instructions/CodeInstruction.vue similarity index 86% rename from src/presentation/components/Code/CodeButtons/Instructions/CodeInstruction.vue rename to src/presentation/components/Code/CodeButtons/Save/Instructions/CodeInstruction.vue index 802b6bb0..6d1131e1 100644 --- a/src/presentation/components/Code/CodeButtons/Instructions/CodeInstruction.vue +++ b/src/presentation/components/Code/CodeButtons/Save/Instructions/CodeInstruction.vue @@ -16,10 +16,10 @@ diff --git a/src/presentation/components/Shared/Hooks/Clipboard/BrowserClipboard.ts b/src/presentation/components/Shared/Hooks/Clipboard/BrowserClipboard.ts new file mode 100644 index 00000000..8211b37f --- /dev/null +++ b/src/presentation/components/Shared/Hooks/Clipboard/BrowserClipboard.ts @@ -0,0 +1,15 @@ +import { Clipboard } from './Clipboard'; + +export type NavigatorClipboard = typeof globalThis.navigator.clipboard; + +export class BrowserClipboard implements Clipboard { + constructor( + private readonly navigatorClipboard: NavigatorClipboard = globalThis.navigator.clipboard, + ) { + + } + + public async copyText(text: string): Promise { + await this.navigatorClipboard.writeText(text); + } +} diff --git a/src/presentation/components/Shared/Hooks/Clipboard/Clipboard.ts b/src/presentation/components/Shared/Hooks/Clipboard/Clipboard.ts new file mode 100644 index 00000000..a4dc64a7 --- /dev/null +++ b/src/presentation/components/Shared/Hooks/Clipboard/Clipboard.ts @@ -0,0 +1,3 @@ +export interface Clipboard { + copyText(text: string): Promise; +} diff --git a/src/presentation/components/Shared/Hooks/Clipboard/UseClipboard.ts b/src/presentation/components/Shared/Hooks/Clipboard/UseClipboard.ts new file mode 100644 index 00000000..3b15acb7 --- /dev/null +++ b/src/presentation/components/Shared/Hooks/Clipboard/UseClipboard.ts @@ -0,0 +1,13 @@ +import { FunctionKeys } from '@/TypeHelpers'; +import { BrowserClipboard } from './BrowserClipboard'; +import { Clipboard } from './Clipboard'; + +export function useClipboard(clipboard: Clipboard = new BrowserClipboard()) { + // Bind functions for direct use from destructured assignments such as `const { .. } = ...`. + const functionKeys: readonly FunctionKeys[] = ['copyText']; + functionKeys.forEach((functionName) => { + const fn = clipboard[functionName]; + clipboard[functionName] = fn.bind(clipboard); + }); + return clipboard; +} diff --git a/src/presentation/components/Shared/Hooks/UseCurrentCode.ts b/src/presentation/components/Shared/Hooks/UseCurrentCode.ts new file mode 100644 index 00000000..6b326713 --- /dev/null +++ b/src/presentation/components/Shared/Hooks/UseCurrentCode.ts @@ -0,0 +1,32 @@ +import { ref } from 'vue'; +import { IApplicationCode } from '@/application/Context/State/Code/IApplicationCode'; +import { IEventSubscriptionCollection } from '@/infrastructure/Events/IEventSubscriptionCollection'; +import { useCollectionState } from './UseCollectionState'; + +export function useCurrentCode( + state: ReturnType, + events: IEventSubscriptionCollection, +) { + const { onStateChange } = state; + + const currentCode = ref(''); + + onStateChange((newState) => { + updateCurrentCode(newState.code.current); + subscribeToCodeChanges(newState.code); + }, { immediate: true }); + + function subscribeToCodeChanges(code: IApplicationCode) { + events.unsubscribeAllAndRegister([ + code.changed.on((newCode) => updateCurrentCode(newCode.code)), + ]); + } + + function updateCurrentCode(newCode: string) { + currentCode.value = newCode; + } + + return { + currentCode, + }; +} diff --git a/src/presentation/components/Shared/Icon/AppIcon.vue b/src/presentation/components/Shared/Icon/AppIcon.vue index a1a1a73d..b9ce644f 100644 --- a/src/presentation/components/Shared/Icon/AppIcon.vue +++ b/src/presentation/components/Shared/Icon/AppIcon.vue @@ -1,5 +1,9 @@