fix(ktable): fix row click handling (#615)

Fix handling of row click events. Changes made: - Ignore clicks from `a`, `button`, and `input` - Ignore clicks from popover content - Update docs
Kong · May 4, 2022 · ea47300 · ea47300
1 parent 1b29ec9
commit ea47300
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 23 deletions.
diff --git 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
@@ -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 {