From 3da777dbb22ae65c66ae6393811ce3771644d8aa Mon Sep 17 00:00:00 2001
From: Phan An <me@phanan.net>
Date: Sun, 16 Jun 2024 17:40:33 +0200
Subject: [PATCH] fix: sidebar toggling on mobile (fr)

---
 .../main-wrapper/sidebar/HomeButton.spec.ts   | 17 +++++++++++++++++
 .../main-wrapper/sidebar/HomeButton.vue       | 16 ++++++++++++++++
 .../layout/main-wrapper/sidebar/Sidebar.vue   | 19 +++----------------
 .../main-wrapper/sidebar/SidebarItem.spec.ts  | 13 +++++++++++--
 .../main-wrapper/sidebar/SidebarItem.vue      |  4 ++++
 .../components/podcast/PodcastListSorter.vue  |  4 ++--
 .../components/ui/SidebarMenuToggleButton.vue | 12 ++++--------
 7 files changed, 57 insertions(+), 28 deletions(-)
 create mode 100644 resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.spec.ts
 create mode 100644 resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.vue

diff --git a/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.spec.ts b/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.spec.ts
new file mode 100644
index 00000000..b776efa6
--- /dev/null
+++ b/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.spec.ts
@@ -0,0 +1,17 @@
+import { screen } from '@testing-library/vue'
+import { expect, it } from 'vitest'
+import UnitTestCase from '@/__tests__/UnitTestCase'
+import { eventBus } from '@/utils'
+import Component from './HomeButton.vue'
+
+new class extends UnitTestCase {
+  protected test () {
+    it('triggers the sidebar toggle event', async () => {
+      this.mock(eventBus, 'emit')
+      this.render(Component)
+      await this.user.click(screen.getByRole('link'))
+
+      expect(eventBus.emit).toHaveBeenCalledWith('TOGGLE_SIDEBAR')
+    })
+  }
+}
diff --git a/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.vue b/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.vue
new file mode 100644
index 00000000..f7a9ee7f
--- /dev/null
+++ b/resources/assets/js/components/layout/main-wrapper/sidebar/HomeButton.vue
@@ -0,0 +1,16 @@
+<template>
+  <a
+    class="bg-black/20 flex items-center px-3.5 rounded-md !text-k-text-secondary hover:!text-k-text-primary"
+    href="#/home"
+    @click="onClick"
+  >
+    <Icon :icon="faHome" fixed-width />
+  </a>
+</template>
+
+<script setup lang="ts">
+import { faHome } from '@fortawesome/free-solid-svg-icons'
+import { eventBus } from '@/utils'
+
+const onClick = () => eventBus.emit('TOGGLE_SIDEBAR')
+</script>
diff --git a/resources/assets/js/components/layout/main-wrapper/sidebar/Sidebar.vue b/resources/assets/js/components/layout/main-wrapper/sidebar/Sidebar.vue
index ddeea260..91a5e5f6 100644
--- a/resources/assets/js/components/layout/main-wrapper/sidebar/Sidebar.vue
+++ b/resources/assets/js/components/layout/main-wrapper/sidebar/Sidebar.vue
@@ -1,18 +1,12 @@
 <template>
     <nav
-      ref="root"
       :class="{ collapsed: !expanded, 'tmp-showing': tmpShowing, showing: mobileShowing }"
       class="flex flex-col fixed md:relative w-full md:w-k-sidebar-width z-10"
       @mouseenter="onMouseEnter"
       @mouseleave="onMouseLeave"
     >
       <section class="home-search-block p-6 flex gap-2">
-        <a
-          class="bg-black/20 flex items-center px-3.5 rounded-md !text-k-text-secondary hover:!text-k-text-primary"
-          href="#/home"
-        >
-          <Icon :icon="faHome" fixed-width />
-        </a>
+        <HomeButton />
         <SearchForm class="flex-1" />
       </section>
 
@@ -31,9 +25,7 @@
 </template>
 
 <script lang="ts" setup>
-import { faHome } from '@fortawesome/free-solid-svg-icons'
 import { computed, ref, watch } from 'vue'
-import { onClickOutside } from '@vueuse/core'
 import { eventBus } from '@/utils'
 import { useAuthorization, useKoelPlus, useLocalStorage, useRouter, useUpload } from '@/composables'
 
@@ -43,6 +35,7 @@ import BtnUpgradeToPlus from '@/components/koel-plus/BtnUpgradeToPlus.vue'
 import SidebarYourMusicSection from './SidebarYourLibrarySection.vue'
 import SidebarManageSection from './SidebarManageSection.vue'
 import SidebarToggleButton from '@/components/layout/main-wrapper/sidebar/SidebarToggleButton.vue'
+import HomeButton from '@/components/layout/main-wrapper/sidebar/HomeButton.vue'
 
 const { onRouteChanged } = useRouter()
 const { isAdmin } = useAuthorization()
@@ -50,7 +43,6 @@ const { allowsUpload } = useUpload()
 const { isPlus } = useKoelPlus()
 const { get: lsGet, set: lsSet } = useLocalStorage()
 
-const root = ref<HTMLElement>()
 const mobileShowing = ref(false)
 const expanded = ref(!lsGet('sidebar-collapsed', false))
 
@@ -83,18 +75,13 @@ const onMouseLeave = (e: MouseEvent) => {
   tmpShowing.value = false
 }
 
-// Not using the <OnClickOutside> component because it'd mess up the overflow/scrolling behavior
-onClickOutside(root, () => (mobileShowing.value = false))
 onRouteChanged(_ => (mobileShowing.value = false))
 
 /**
  * Listen to toggle sidebar event to show or hide the sidebar.
  * This should only be triggered on a mobile device.
  */
-eventBus.on('TOGGLE_SIDEBAR', () => {
-  console.log(mobileShowing.value)
-  mobileShowing.value = !mobileShowing.value
-})
+eventBus.on('TOGGLE_SIDEBAR', () => (mobileShowing.value = !mobileShowing.value))
 </script>
 
 <style lang="postcss" scoped>
diff --git a/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.spec.ts b/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.spec.ts
index 362b4862..5df20392 100644
--- a/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.spec.ts
+++ b/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.spec.ts
@@ -2,7 +2,8 @@ import { expect, it } from 'vitest'
 import UnitTestCase from '@/__tests__/UnitTestCase'
 import { screen } from '@testing-library/vue'
 import { faHome } from '@fortawesome/free-solid-svg-icons'
-import SidebarItem from './SidebarItem.vue'
+import Component from './SidebarItem.vue'
+import { eventBus } from '@/utils'
 
 new class extends UnitTestCase {
   protected test () {
@@ -18,10 +19,18 @@ new class extends UnitTestCase {
 
       expect(screen.getByTestId('sidebar-item').classList.contains('current')).toBe(true)
     })
+
+    it ('emits the sidebar toggle event when clicked', async () => {
+      const mock  = this.mock(eventBus, 'emit')
+      this.renderComponent()
+      await this.user.click(screen.getByTestId('sidebar-item'))
+
+      expect(mock).toHaveBeenCalledWith('TOGGLE_SIDEBAR')
+    })
   }
 
   private renderComponent () {
-    return this.render(SidebarItem, {
+    return this.render(Component, {
       props: {
         icon: faHome,
         href: '#',
diff --git a/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.vue b/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.vue
index 23c14822..ff8b60e0 100644
--- a/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.vue
+++ b/resources/assets/js/components/layout/main-wrapper/sidebar/SidebarItem.vue
@@ -4,6 +4,7 @@
     class="relative before:right-0 px-6 before:top-1/4 before:w-[4px] before:h-1/2 before:absolute before:rounded-full
     before:transition-[box-shadow,_background-color] before:ease-in-out before:duration-500"
     data-testid="sidebar-item"
+    @click="onClick"
   >
     <a
       :href="props.href"
@@ -24,6 +25,7 @@
 <script lang="ts" setup>
 import { ref } from 'vue'
 import { useRouter } from '@/composables'
+import { eventBus } from '@/utils'
 
 const props = withDefaults(defineProps<{ href?: string | undefined; screen?: ScreenName | undefined }>(), {
   href: undefined,
@@ -34,6 +36,8 @@ const current = ref(false)
 
 const { onRouteChanged } = useRouter()
 
+const onClick = () => eventBus.emit('TOGGLE_SIDEBAR')
+
 if (screen) {
   onRouteChanged(route => (current.value = route.screen === props.screen))
 }
diff --git a/resources/assets/js/components/podcast/PodcastListSorter.vue b/resources/assets/js/components/podcast/PodcastListSorter.vue
index 5a3b0d51..91ac7672 100644
--- a/resources/assets/js/components/podcast/PodcastListSorter.vue
+++ b/resources/assets/js/components/podcast/PodcastListSorter.vue
@@ -32,8 +32,8 @@
 <script setup lang="ts">
 import { faArrowDown, faArrowUp } from '@fortawesome/free-solid-svg-icons'
 import { OnClickOutside } from '@vueuse/components'
-import { useFloatingUi } from '@/composables'
 import { computed, onBeforeUnmount, onMounted, ref, toRefs } from 'vue'
+import { useFloatingUi } from '@/composables'
 
 const props = withDefaults(defineProps<{
   field?: PodcastListSortField
@@ -63,7 +63,7 @@ const items: { label: string, field: PodcastListSortField }[] = [
   { label: 'Author', field: 'author' }
 ]
 
-const currentFieldLabel = computed(() => items.find(item => item.field === activeField.value).label)
+const currentFieldLabel = computed(() => items.find(item => item.field === activeField.value)?.label)
 
 const sort = (field: MaybeArray<PodcastListSortField>) => {
   emit('sort', field)
diff --git a/resources/assets/js/components/ui/SidebarMenuToggleButton.vue b/resources/assets/js/components/ui/SidebarMenuToggleButton.vue
index b89daeeb..9278f209 100644
--- a/resources/assets/js/components/ui/SidebarMenuToggleButton.vue
+++ b/resources/assets/js/components/ui/SidebarMenuToggleButton.vue
@@ -7,18 +7,14 @@
 
 <script lang="ts" setup>
 import { faBars, faTimes } from '@fortawesome/free-solid-svg-icons'
+import { ref } from 'vue'
 import { eventBus } from '@/utils'
 
 import ExtraDrawerButton from '@/components/layout/main-wrapper/extra-drawer/ExtraDrawerButton.vue'
-import { ref } from 'vue'
 
 const sidebarExpanded = ref(false)
 
-const toggleSidebar = () => {
-  // Since the sidebar will be hidden if we click outside the menu anyway, we don't actually need
-  // to emit the event to collapse it. We just need to toggle the state to show the
-  // icon accordingly.
-  if (!sidebarExpanded.value) eventBus.emit('TOGGLE_SIDEBAR')
-  sidebarExpanded.value = !sidebarExpanded.value
-}
+eventBus.on('TOGGLE_SIDEBAR', () => (sidebarExpanded.value = !sidebarExpanded.value))
+
+const toggleSidebar = () => eventBus.emit('TOGGLE_SIDEBAR')
 </script>