fix(ui5-table): rowClick is fired for popped in cells as well (#1671)

SAP · May 21, 2020 · c48f541 · c48f541
1 parent da30bc1
commit c48f541
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 72 deletions.
diff --git a/packages/main/src/Table.js b/packages/main/src/Table.js
@@ -201,7 +201,6 @@ class Table extends UI5Element {
 		}.bind(this);
 
 		this.fnOnRowFocused = this.onRowFocused.bind(this);
-		this.fnOnRowClick = this.onRowClick.bind(this);
 
 		this._handleResize = this.popinContent.bind(this);
 	}
@@ -218,9 +217,6 @@ class Table extends UI5Element {
 
 			row.removeEventListener("ui5-_focused", this.fnOnRowFocused);
 			row.addEventListener("ui5-_focused", this.fnOnRowFocused);
-
-			row.removeEventListener("ui5-_click", this.fnOnRowClick);
-			row.addEventListener("ui5-_click", this.fnOnRowClick);
 		});
 
 		this.visibleColumns = this.columns.filter((column, index) => {
@@ -244,10 +240,6 @@ class Table extends UI5Element {
 		this._itemNavigation.update(event.target);
 	}
 
-	onRowClick(event) {
-		this.fireEvent("rowClick", { row: event.target });
-	}
-
 	_onColumnHeaderClick(event) {
 		this.getColumnHeader().focus();
 		this._itemNavigation.update(event.target);

diff --git a/packages/main/src/TableCell.hbs b/packages/main/src/TableCell.hbs
@@ -1,5 +1,4 @@
 <td
-	@click="{{_onclick}}"
 	tabindex="-1"
 	part="cell"
 >

diff --git a/packages/main/src/TableCell.js b/packages/main/src/TableCell.js
@@ -79,10 +79,6 @@ class TableCell extends UI5Element {
 	static get template() {
 		return TableCellTemplate;
 	}
-
-	_onclick(event) {
-		this.fireEvent("_cellclick", event);
-	}
 }
 
 TableCell.define();

diff --git a/packages/main/src/TableRow.hbs b/packages/main/src/TableRow.hbs
@@ -2,7 +2,7 @@
 	class="ui5-table-row-root"
 	tabindex="{{_tabIndex}}"
 	@focusin="{{_onfocusin}}"
-	@ui5-_cellclick="{{_oncellclick}}"
+	@click="{{_onrowclick}}"
 	data-sap-focus-ref
 	part="row"
 >
@@ -19,7 +19,7 @@
 
 {{#if shouldPopin}}
 	{{#each popinCells}}
-		<tr part="popin-row" class="ui5-table-popin-row" @ui5-_cellclick="{{_oncellclick}}">
+		<tr part="popin-row" class="ui5-table-popin-row" @click="{{../_onrowclick}}">
 			<td colspan="{{../visibleCellsCount}}">
 				{{#if this.popinText}}
 					<span class="ui5-table-row-popin-title">{{this.popinText}}:</span>

diff --git a/packages/main/src/TableRow.js b/packages/main/src/TableRow.js
@@ -39,8 +39,6 @@ const metadata = {
 	},
 	events: /** @lends sap.ui.webcomponents.main.TableRow.prototype */ {
 		_focused: {},
-
-		_click: {},
 	},
 };
 
@@ -59,11 +57,6 @@ const metadata = {
  * @public
  */
 class TableRow extends UI5Element {
-	constructor() {
-		super();
-		this.fnOnCellClick = this._oncellclick.bind(this);
-	}
-
 	static get metadata() {
 		return metadata;
 	}
@@ -88,7 +81,7 @@ class TableRow extends UI5Element {
 		this.fireEvent("_focused", event);
 	}
 
-	_oncellclick(event) {
+	_onrowclick(event) {
 		if (this._getActiveElementTagName() === "body") {
 			// If the user clickes on non-focusable element within the ui5-table-cell,
 			// the focus goes to the body, se we have to bring it back to the row.
@@ -97,7 +90,7 @@ class TableRow extends UI5Element {
 			this._onfocusin(event, true /* force row focus */);
 		}
 
-		this.fireEvent("_click");
+		this.fireEvent("rowClick", { row: this });
 	}
 
 	_getActiveElementTagName() {

diff --git a/packages/main/test/pages/TableCustomStyling.html b/packages/main/test/pages/TableCustomStyling.html
@@ -33,39 +33,39 @@
 		height: 100%;
 	}
 
-    /* All rows - height */
-    #tbl ui5-table-row::part(row) {
-        height: 4rem;
-    }
-    /* One row - higher than the rest */
-    #tbl ui5-table-row#row3::part(row) {
-        height: 6rem;
-    }
-
-    /* All columns - center both vertically and horizontally */
-    #tbl ui5-table-column::part(column) {
-        vertical-align: middle;
-        text-align: center;
-    }
-
-    /* All cells - center both vertically and horizontally */
-    #tbl ui5-table-cell::part(cell) {
-        vertical-align: middle;
-        text-align: center;
-    }
-
-    /* Title column & cells - left aligned */
-    #tbl ui5-table-cell.title-cell::part(cell),
-    #tbl ui5-table-column.title-column::part(column) {
-        text-align: left;
-    }
-
-    /* Price column & cells - right aligned with some padding */
-    #tbl ui5-table-cell.price-cell::part(cell),
-    #tbl ui5-table-column.price-column::part(column) {
-        text-align: right;
-        padding-right: 0.75rem;
-    }
+	/* All rows - height */
+	#tbl ui5-table-row::part(row) {
+		height: 4rem;
+	}
+	/* One row - higher than the rest */
+	#tbl ui5-table-row#row3::part(row) {
+		height: 6rem;
+	}
+
+	/* All columns - center both vertically and horizontally */
+	#tbl ui5-table-column::part(column) {
+		vertical-align: middle;
+		text-align: center;
+	}
+
+	/* All cells - center both vertically and horizontally */
+	#tbl ui5-table-cell::part(cell) {
+		vertical-align: middle;
+		text-align: center;
+	}
+
+	/* Title column & cells - left aligned */
+	#tbl ui5-table-cell.title-cell::part(cell),
+	#tbl ui5-table-column.title-column::part(column) {
+		text-align: left;
+	}
+
+	/* Price column & cells - right aligned with some padding */
+	#tbl ui5-table-cell.price-cell::part(cell),
+	#tbl ui5-table-column.price-column::part(column) {
+		text-align: right;
+		padding-right: 0.75rem;
+	}
 </style>
 
 <body style="background-color: var(--sapBackgroundColor);">
@@ -103,24 +103,30 @@
 			<ui5-table-cell class="price-cell">956 EUR</ui5-table-cell>
 		</ui5-table-row>
 
-        <ui5-table-row id="row2">
-            <ui5-table-cell class="title-cell">Notebook Basic 17</ui5-table-cell>
-            <ui5-table-cell>Very Best Screens</ui5-table-cell>
-            <ui5-table-cell>40 x 18 x 3 cm</ui5-table-cell>
-            <ui5-table-cell>4.6 KG</ui5-table-cell>
-            <ui5-table-cell class="price-cell">1956 EUR</ui5-table-cell>
-        </ui5-table-row>
-
-        <ui5-table-row id="row3">
-            <ui5-table-cell class="title-cell">Notebook Basic 19</ui5-table-cell>
-            <ui5-table-cell>Very Best Screens</ui5-table-cell>
-            <ui5-table-cell>50 x 18 x 3 cm</ui5-table-cell>
-            <ui5-table-cell>4.9 KG</ui5-table-cell>
-            <ui5-table-cell class="price-cell">2956 EUR</ui5-table-cell>
-        </ui5-table-row>
+		<ui5-table-row id="row2">
+			<ui5-table-cell class="title-cell">Notebook Basic 17</ui5-table-cell>
+			<ui5-table-cell>Very Best Screens</ui5-table-cell>
+			<ui5-table-cell>40 x 18 x 3 cm</ui5-table-cell>
+			<ui5-table-cell>4.6 KG</ui5-table-cell>
+			<ui5-table-cell class="price-cell">1956 EUR</ui5-table-cell>
+		</ui5-table-row>
+
+		<ui5-table-row id="row3">
+			<ui5-table-cell class="title-cell">Notebook Basic 19</ui5-table-cell>
+			<ui5-table-cell>Very Best Screens</ui5-table-cell>
+			<ui5-table-cell>50 x 18 x 3 cm</ui5-table-cell>
+			<ui5-table-cell>4.9 KG</ui5-table-cell>
+			<ui5-table-cell class="price-cell">2956 EUR</ui5-table-cell>
+		</ui5-table-row>
 
 	</ui5-table>
 
+	<script>
+		document.getElementById("tbl").addEventListener("rowClick", function(event) {
+			console.log("Row click: ", event.detail.row);
+		});
+	</script>
+
 </body>
 
 </html>