From e269cee11bb5e0cdc243a4b02e7d766b58228bd3 Mon Sep 17 00:00:00 2001
From: "kai.arrowood" <kai.arrowood@konghq.com>
Date: Wed, 4 May 2022 12:10:58 -0400
Subject: [PATCH] fix(ktable): fix row click handling

---
 docs/components/table.md         | 105 ++++++++++++++++++++++++-------
 src/components/KTable/KTable.vue |   6 +-
 2 files changed, 88 insertions(+), 23 deletions(-)

diff --git a/docs/components/table.md b/docs/components/table.md
index fdf868a1d1..d1d452d7b9 100644
--- a/docs/components/table.md
+++ b/docs/components/table.md
@@ -537,49 +537,58 @@ export default {
 
 ## Events
 
-Bind any DOM [events](https://developer.mozilla.org/en-US/docs/Web/Events) to various parts of the table. We support events on both table rows and cells in addition to click elements inside a row (ie. buttons or hyperlinks) without triggering the a row or cell event. You can also add logic to check for `metakey` to support cmd/ctrl when clicking. Examples highlighted below.
+Bind any DOM [events](https://developer.mozilla.org/en-US/docs/Web/Events) to various parts of the table. We support events on both table rows and cells, but must be careful with clickable content in rows when row click is enabled. You can also add logic to check for `metakey` to support cmd/ctrl when clicking. Examples highlighted below.
 
 ### Rows
 
 `@row:{event}` - returns the `Event`, the row item, and the type: `row`
 
-```vue
-  <KTable @row:click="rowHandler" @row:dblclick="rowHandler" />
-```
+To avoid firing row clicks by accident, the row click handler ignores events coming from `a`, `button`, `input`, and `select` elements (unless they have the `disabled` attribute). As such click handlers attached to these element types do not require stopping propagation via `@click.stop`.
 
 <KTable :headers="tableOptionsLinkHeaders" :fetcher="tableOptionsLinkFetcher" @row:click="handleRowClick">
   <template v-slot:company="{ rowValue }">
-    <a v-if="rowValue" @click="linkHander">{{ rowValue.label }}</a>
-    <span v-else>{{ rowValue }}</span>
+    <a @click="linkHander">{{rowValue.label}}</a>
   </template>
   <template v-slot:actions>
     <div class="float-right">
       <KButton appearance="secondary" @click="buttonHandler">
-        Visit Website
+        Fire Button Handler!
       </KButton>
     </div>
   </template>
+  <template v-slot:input>
+    <KInput type="text" placeholder="Need help?" />
+    <KInput type="text" placeholder="Row click me" disabled />
+  </template>
 </KTable>
 
-```vue
+```html
 <KTable
   :fetcher="fetcher"
   :headers="headers"
   @row:click="handleRowClick">
   <template v-slot:company="{rowValue}">
-    <a v-if="rowValue" @click="linkHander">{{rowValue.label}}</a>
-    <span v-else>{{rowValue}}</span>
+    <!-- .stop not needed on @click because we ignore clicks from anchors -->
+    <a @click="linkHander">{{rowValue.label}}</a>
   </template>
   <template v-slot:actions>
     <div class="float-right">
+      <!-- .stop not needed on @click because we ignore clicks from buttons -->
       <KButton
         appearance="secondary"
         @click="buttonHandler">
-        Visit Website
+        Fire Button Handler!
       </KButton>
     </div>
   </template>
+  <template v-slot:input>
+    <!-- no special handling needed because click events on input elements are ignored -->
+    <KInput type="text" placeholder="Need help?" />
+    <!-- row click is triggered when clicking disabled elements -->
+    <KInput type="text" placeholder="Row click me" disabled />
+  </template>
 </KTable>
+
 <script>
 export default {
   data() {
@@ -593,9 +602,9 @@ export default {
       const metaKeyPressed = e.metaKey || e.ctrlKey
 
       if (metaKeyPressed) {
-        return window.open(row.company.href)
+        this.$toaster.open('MetaKey row click')
       } else {
-        window.location = row.company.href
+        this.$toaster.open('Row click event fired!')
       }
     },
     linkHander (e) {
@@ -609,14 +618,62 @@ export default {
 </script>
 ```
 
-### Cells
+Click events tied to non-ignored elements (not `a`, `button`, `input`, `select`) must use the `.stop` keyword to stop propagation firing the row click handler.
 
-`@cell:{event}` - returns the `Event`, the cell value, and the type: `cell`
+Using a `KPop` inside of a clickable row requires some special handling. Non-clickable content must be wrapped in a `div` with the `@click.stop` attribute to prevent the row click handler from firing if a user clicks content inside of the popover. Any handlers on non-ignored elements will need to have `.stop`.
+
+<KTable :headers="tableOptionsLinkHeaders2" :fetcher="tableOptionsLinkFetcher" @row:click="handleRowClick">
+  <template v-slot:company="{rowValue}">
+    <a @click="linkHander">{{rowValue.label}}</a>
+  </template>
+  <template v-slot:wrapped>
+    <div>Row click event <div class="d-inline-block" @click.stop="buttonHandler"><KBadge appearance="success">Button click</KBadge></div></div>
+  </template>
+  <template v-slot:other>
+    <div>
+      <KPop title="Cool header">
+        <KButton>Popover!</KButton>
+        <template v-slot:content>
+          <div @click.stop>Clicking me does nothing!</div>
+          <div class="d-inline-block" @click.stop="buttonHandler"><KBadge appearance="success">Button click</KBadge></div>
+        </template>
+      </KPop>
+    </div>
+  </template>
+</KTable>
 
 ```vue
-<KTable @cell:mouseout="cellHandler" @cell:mousedown="cellHandler" />
+<KTable
+  :fetcher="fetcher"
+  :headers="headers"
+  @row:click="handleRowClick">
+  <template v-slot:company="{rowValue}">
+    <a @click="linkHander">{{rowValue.label}}</a>
+  </template>
+  <template v-slot:wrapped>
+    <!-- We have a click event on a div, div clicks are not ignored so we need .stop -->
+    <div>Row click event <div @click.stop="buttonHandler"><KBadge appearance="success">Button click</KBadge></div></div>
+  </template>
+  <template v-slot:other>
+    <div>
+      <KPop title="Cool header">
+        <KButton>Popover!</KButton>
+        <template v-slot:content>
+          <!-- non-clickable content in a KPop MUST be wrapped in a div with @click.stop to prevent row click firing on content click -->
+          <div @click.stop>Clicking me does nothing!</div>
+          <!-- an example where we want a click event to fire from the popover, requires .stop on the @click -->
+          <div @click.stop="buttonHandler"><KBadge appearance="success">Button click</KBadge></div>
+        </template>
+      </KPop>
+    </div>
+  </template>
+</KTable>
 ```
 
+### Cells
+
+`@cell:{event}` - returns the `Event`, the cell value, and the type: `cell`
+
 <template>
   <div>
     <div v-if="eventType">
@@ -1149,8 +1206,14 @@ export default defineComponent({
       ],
       tableOptionsLinkHeaders: [
         { label: 'Company', key: 'company' },
+        { label: 'Input', key: 'input' },
         { key: 'actions', hideLabel: true }
       ],
+      tableOptionsLinkHeaders2: [
+        { label: 'Company', key: 'company' },
+        { label: 'Wrapped', key: 'wrapped' },
+        { label: 'Pop', key: 'other' }
+      ],
       tableOptionsCellAttrsHeaders: [
         { label: 'Name', key: 'name' },
         { label: 'Company', key: 'company' },
@@ -1363,12 +1426,10 @@ export default defineComponent({
     handleRowClick(e, row) {
       const metaKeyPressed = e.metaKey || e.ctrlKey
 
-      if (e.target.tagName !== 'BUTTON') {
-        if (metaKeyPressed) {
-          return window.open(row.company.href)
-        } else {
-          window.location = row.company.href
-        }
+      if (metaKeyPressed) {
+        this.$toaster.open('MetaKey row click')
+      } else {
+        this.$toaster.open('Row click event fired!')
       }
     },
     linkHander (e) {
diff --git a/src/components/KTable/KTable.vue b/src/components/KTable/KTable.vue
index 6d42d17d70..ee918ef6d6 100644
--- a/src/components/KTable/KTable.vue
+++ b/src/components/KTable/KTable.vue
@@ -544,6 +544,7 @@ export default defineComponent({
       return (entity: any, rowData: any) => {
         const rowListeners = pluckListeners('onRow:', attrs)(rowData, 'row')
         const cellListeners = pluckListeners('onCell:', attrs)(entity, 'cell')
+        const ignoredElements = ['a', 'button', 'input', 'select']
 
         if (rowListeners.click) {
           isClickable.value = true
@@ -553,7 +554,10 @@ export default defineComponent({
           ...rowListeners,
           ...cellListeners,
           click(e: any) {
-            if (rowListeners.click || cellListeners.click) {
+            const isPopoverContent = e.target.className.includes('k-popover')
+            // ignore click if it is from the popover, or is a non-disabled ignored element
+            if ((!ignoredElements.includes(e.target.tagName.toLowerCase()) || e.target.hasAttribute('disabled')) &&
+                 !isPopoverContent && (rowListeners.click || cellListeners.click)) {
               if (cellListeners.click) {
                 cellListeners.click(e, entity, 'cell')
               } else {