From 6e6c5a2f021df7fcf2f0d40e7aba8e83a0f146e9 Mon Sep 17 00:00:00 2001
From: Shivam Mishra <scm.mymail@gmail.com>
Date: Thu, 31 Oct 2024 09:39:18 +0530
Subject: [PATCH] refactor: use css only last item detection (#10363)

The last item in the sidebar top level group has an indicator specified,
the problem in our case is that the structure can be nested and have sub
groups. So selecting the last item correctly can be tricky.

Previous implementation relied on the using DOM queries to find the last
item from a flat list of children, it would trigger on a `watch`. This
was error-prone as well as non idiomatic. The new approach is CSS-only
and reduces the unnecessary compute required.

Codepen for reference: https://codepen.io/scmmishra/pen/yLmKNLW

---------

Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
---
 .../components-next/sidebar/SidebarGroup.vue  | 74 ++++++++++++++-----
 .../sidebar/SidebarGroupLeaf.vue              | 41 ----------
 2 files changed, 57 insertions(+), 58 deletions(-)

diff --git a/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue b/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue
index f4ffcd0cd..89d99d402 100644
--- a/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue
+++ b/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue
@@ -1,5 +1,5 @@
 <script setup>
-import { computed, watch, ref } from 'vue';
+import { computed } from 'vue';
 import { useSidebarContext } from './provider';
 import { useRoute, useRouter } from 'vue-router';
 import Policy from 'dashboard/components/policy.vue';
@@ -26,17 +26,6 @@ const {
   isAllowed,
 } = useSidebarContext();
 
-const parentEl = ref(null);
-
-const locateLastChild = async () => {
-  const children = parentEl.value?.querySelectorAll('.child-item');
-  if (!children) return;
-
-  children.forEach((child, index) => {
-    child.classList.toggle('last-child-item', index === children.length - 1);
-  });
-};
-
 const navigableChildren = computed(() => {
   return props.children?.flatMap(child => child.children || child) || [];
 });
@@ -103,10 +92,6 @@ const toggleTrigger = () => {
   }
   setExpandedItem(props.name);
 };
-
-watch([expandedItem, accessibleItems], locateLastChild, {
-  immediate: true,
-});
 </script>
 
 <!-- eslint-disable-next-line vue/no-root-v-if -->
@@ -132,7 +117,6 @@ watch([expandedItem, accessibleItems], locateLastChild, {
     <ul
       v-if="hasChildren"
       v-show="isExpanded || hasActiveChild"
-      ref="parentEl"
       class="list-none m-0 grid sidebar-group-children"
     >
       <template v-for="child in children" :key="child.name">
@@ -155,3 +139,59 @@ watch([expandedItem, accessibleItems], locateLastChild, {
     </ul>
   </Policy>
 </template>
+
+<style>
+.sidebar-group-children .child-item::before {
+  content: '';
+  position: absolute;
+  width: 0.125rem;
+  /* 0.5px */
+  height: 100%;
+}
+
+.sidebar-group-children .child-item:first-child::before {
+  border-radius: 4px 4px 0 0;
+}
+
+/* This selects the last child in a group */
+/* https://codepen.io/scmmishra/pen/yLmKNLW */
+.sidebar-group-children > .child-item:last-child::before,
+.sidebar-group-children
+  > *:last-child
+  > *:last-child
+  > .child-item:last-child::before {
+  height: 20%;
+}
+
+.sidebar-group-children > .child-item:last-child::after,
+.sidebar-group-children
+  > *:last-child
+  > *:last-child
+  > .child-item:last-child::after {
+  content: '';
+  position: absolute;
+  width: 10px;
+  height: 12px;
+  bottom: calc(50% - 2px);
+  border-bottom-width: 0.125rem;
+  border-left-width: 0.125rem;
+  border-right-width: 0px;
+  border-top-width: 0px;
+  border-radius: 0 0 0 4px;
+  left: 0;
+}
+
+.app-rtl--wrapper .sidebar-group-children > .child-item:last-child::after,
+.app-rtl--wrapper
+  .sidebar-group-children
+  > *:last-child
+  > *:last-child
+  > .child-item:last-child::after {
+  right: 0;
+  border-bottom-width: 0.125rem;
+  border-right-width: 0.125rem;
+  border-left-width: 0px;
+  border-top-width: 0px;
+  border-radius: 0 0 4px 0px;
+}
+</style>
diff --git a/app/javascript/dashboard/components-next/sidebar/SidebarGroupLeaf.vue b/app/javascript/dashboard/components-next/sidebar/SidebarGroupLeaf.vue
index ec630d2a5..13bdb040d 100644
--- a/app/javascript/dashboard/components-next/sidebar/SidebarGroupLeaf.vue
+++ b/app/javascript/dashboard/components-next/sidebar/SidebarGroupLeaf.vue
@@ -49,44 +49,3 @@ const shouldRenderComponent = computed(() => {
     </component>
   </Policy>
 </template>
-
-<style scoped>
-.child-item::before {
-  content: '';
-  position: absolute;
-  width: 0.125rem;
-  /* 0.5px */
-  height: 100%;
-}
-
-.child-item:first-child::before {
-  border-radius: 4px 4px 0 0;
-}
-
-.last-child-item::before {
-  height: 20%;
-}
-
-.last-child-item::after {
-  content: '';
-  position: absolute;
-  width: 10px;
-  height: 12px;
-  bottom: calc(50% - 2px);
-  border-bottom-width: 0.125rem;
-  border-left-width: 0.125rem;
-  border-right-width: 0px;
-  border-top-width: 0px;
-  border-radius: 0 0 0 4px;
-  left: 0;
-}
-
-.app-rtl--wrapper .last-child-item::after {
-  right: 0;
-  border-bottom-width: 0.125rem;
-  border-right-width: 0.125rem;
-  border-left-width: 0px;
-  border-top-width: 0px;
-  border-radius: 0 0 4px 0px;
-}
-</style>