Skip to content

fix(ui5-table): fix row navigation and focus handling #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 1, 2019
Merged

fix(ui5-table): fix row navigation and focus handling #876

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1d0950b
fix(ui5-table): row navigation and focus handling
ilhan007 Oct 22, 2019
a1c208a
Merge branch 'master' into fix-tabble-navigation
ilhan007 Oct 29, 2019
36c073f
rems instead of px
ilhan007 Oct 29, 2019
540fe24
remove pointer-events: none
ilhan007 Oct 29, 2019
ea4b691
attach for cell click in template
ilhan007 Oct 30, 2019
1d323a5
include column header in row navigation
ilhan007 Oct 30, 2019
721c492
fix js error when column header is not yet rendered
ilhan007 Oct 30, 2019
5b1ba0d
simplify if condition
ilhan007 Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/main/src/Table.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<table border="0" cellspacing="0" cellpadding="0">

<thead>
<tr class="ui5-table-header-row" style="height: 48px">
<tr id="{{_id}}-columnHeader" class="ui5-table-header-row" tabindex="0" style="height: 48px" @click="{{onColumnHeaderClick}}">
{{#each visibleColumns}}
<slot name="{{this._individualSlot}}"></slot>
{{/each}}
Expand Down
4 changes: 4 additions & 0 deletions packages/main/src/Table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ class Table extends UI5Element {
}
}

onColumnHeaderClick() {
this.getDomRef().querySelector(`#${this._id}-columnHeader`).focus();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wish we make this part of the item navigation but its fine for now

Copy link
Member Author

@ilhan007 ilhan007 Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

popinContent(_event) {
const clientRect = this.getDomRef().getBoundingClientRect();
const tableWidth = clientRect.width;
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TableCell.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<td>
<td tabindex="-1">
<slot></slot>
</td>
13 changes: 8 additions & 5 deletions packages/main/src/TableRow.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
<tr class="ui5-table-row-root">
{{#each visibleCells}}
<slot name="{{this._individualSlot}}"></slot>
{{/each}}
<tr
class="ui5-table-row-root"
tabindex="{{_tabIndex}}"
@focusin="{{_onfocusin}}"
data-sap-focus-ref>
{{#each visibleCells}}
<slot name="{{this._individualSlot}}"></slot>
{{/each}}
</tr>


{{#each popinCells}}
<tr class="ui5-table-popin-row">
<td colspan="{{../visibleCellsCount}}">

<span class="ui5-table-row-popin-title">{{this.popinText}}:</span>
<div>
<slot name="{{this.cell._individualSlot}}"></slot>
Expand Down
36 changes: 32 additions & 4 deletions packages/main/src/TableRow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ const metadata = {
* @public
*/
class TableRow extends UI5Element {
constructor() {
super();
this.fnOnCellClick = this._oncellclick.bind(this);
}

static get metadata() {
return metadata;
}
Expand All @@ -72,6 +77,28 @@ class TableRow extends UI5Element {
return TableRowTemplate;
}

_onfocusin(event, forceSelfFocus = false) {
if (forceSelfFocus || this._getActiveElementTagName() === "ui5-table-cell") {
this.getDomRef().focus();
}

this.fireEvent("_focused", event);
}

_oncellclick(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.
// If the uset clicks on input, button or similar clickable element,
// the focus remains on that element.
this._onfocusin(event, true /* force row focus */);
}
}

_getActiveElementTagName() {
return document.activeElement.localName.toLocaleLowerCase();
}

onBeforeRendering() {
this.visibleCells = [];
this.popinCells = [];
Expand All @@ -80,6 +107,11 @@ class TableRow extends UI5Element {
return;
}

this.cells.forEach(cell => {
cell.removeEventListener("click", this.fnOnCellClick);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you better fire some event from the cell and rely on bubbling so you can catch it in the Row

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

cell.addEventListener("click", this.fnOnCellClick);
});

this._columnsInfo.forEach((info, index) => {
const cell = this.cells[index];

Expand Down Expand Up @@ -128,10 +160,6 @@ class TableRow extends UI5Element {
get visibleCellsCount() {
return this.visibleCells.length;
}

onfocusin(event) {
this.fireEvent("_focused", event);
}
}

TableRow.define();
Expand Down
5 changes: 5 additions & 0 deletions packages/main/src/themes/Table.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ table {
font-size: var(--sapMFontMediumSize);
}

.ui5-table-header-row:focus {
outline: var(--ui5_table_header_row_outline_width) dotted var(--sapUiContentFocusColor);
outline-offset: -0.125rem;
}

tr {
height: 3rem;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/main/src/themes/TableRow.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
border-top: 1px solid var(--sapUiListBorderColor);
}

.ui5-table-row-root:focus {
outline: var(--ui5_table_row_outline_width) dotted var(--sapUiContentFocusColor);
outline-offset: -0.125rem;
}

.ui5-table-popin-row td {
padding: .25rem;
padding-left: 1rem;
Expand Down
4 changes: 3 additions & 1 deletion packages/main/src/themes/base/Table-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
:root {}
:root {
--ui5_table_header_row_outline_width: 1px;
}
3 changes: 3 additions & 0 deletions packages/main/src/themes/base/TableRow-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:root {
--ui5_table_row_outline_width: 1px;
}
1 change: 1 addition & 0 deletions packages/main/src/themes/sap_belize/TableRow-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../base/TableRow-parameters.css";
1 change: 1 addition & 0 deletions packages/main/src/themes/sap_belize/parameters-bundle.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
@import "./Switch-parameters.css";
@import "./TabContainer-parameters.css";
@import "./Table-parameters.css";
@import "./TableRow-parameters.css";
@import "./TextArea-parameters.css";
@import "./TimelineItem-parameters.css";
@import "./ToggleButton-parameters.css";
Expand Down
6 changes: 5 additions & 1 deletion packages/main/src/themes/sap_belize_hcb/Table-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
@import "../base/Table-parameters.css";
@import "../base/Table-parameters.css";

:root {
--ui5_table_header_row_outline_width: 0.125rem;
}
5 changes: 5 additions & 0 deletions packages/main/src/themes/sap_belize_hcb/TableRow-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import "../base/TableRow-parameters.css";

:root {
--ui5_table_row_outline_width: 0.125rem;
}
1 change: 1 addition & 0 deletions packages/main/src/themes/sap_belize_hcb/parameters-bundle.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
@import "./Switch-parameters.css";
@import "./TabContainer-parameters.css";
@import "./Table-parameters.css";
@import "./TableRow-parameters.css";
@import "./TextArea-parameters.css";
@import "./TimelineItem-parameters.css";
@import "./ToggleButton-parameters.css";
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/themes/sap_fiori_3/TableRow-parameters.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../base/TableRow-parameters.css";
1 change: 1 addition & 0 deletions packages/main/src/themes/sap_fiori_3/parameters-bundle.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@
@import "./YearPicker-parameters.css";
@import "./CalendarHeader-parameters.css";
@import "./Table-parameters.css";
@import "./TableRow-parameters.css";
@import "./Token-parameters.css";
@import "./MultiComboBox-parameters.css";
10 changes: 5 additions & 5 deletions packages/main/test/pages/Table.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,14 @@
</div>
</ui5-table-column>

<ui5-table-row>
<ui5-table-cell>Dublin</ui5-table-cell>
<ui5-table-row >
<ui5-table-cell><span>Dublin</span> <ui5-button>click</ui5-button></ui5-table-cell>
</ui5-table-row>
<ui5-table-row>
<ui5-table-cell>Sofia</ui5-table-cell>
<ui5-table-cell><ui5-label for="myInput">Sofia</ui5-label> <ui5-input id="myInput">click</ui5-input></ui5-table-cell>
</ui5-table-row>
<ui5-table-row>
<ui5-table-cell>London</ui5-table-cell>
<ui5-table-row >
<ui5-table-cell>London <input /> </ui5-table-cell>
</ui5-table-row>
</ui5-table>

Expand Down