Skip to content
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

Improve the reverse navigation order of focusable elements within KTable cells #840

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Open

Improve the reverse navigation order of focusable elements within KTable cells #840

Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a4188c8
refactor the handlekeydown code
shivam-daksh Oct 20, 2024
1bb1471
lint fix by commenting the unused code
shivam-daksh Oct 21, 2024
75f07a2
row highlight fix when tab key pressed
shivam-daksh Oct 23, 2024
6163a15
Fix the infinite loop of Tab key over table cells
shivam-daksh Oct 25, 2024
c518276
adding the lost comments during refactoring
shivam-daksh Oct 27, 2024
f41e333
Merge branch 'learningequality:develop' into refactor-handlekeydown
shivam-daksh Nov 15, 2024
b6f712e
fixing the tab and shift+tab key functionality
shivam-daksh Nov 16, 2024
0708628
highlight rows and columns
shivam-daksh Nov 17, 2024
fff71d4
tab and shift+tab working but first cell of table not getting focused
shivam-daksh Nov 17, 2024
4b61140
Merge branch 'refactor-handlekeydown' of https://github.com/shivam-da…
shivam-daksh Nov 17, 2024
59e33b9
sort the columns only from header row
shivam-daksh Nov 20, 2024
0f5bd96
utilize updateFocusState function in arrow keys handling
shivam-daksh Nov 20, 2024
354df74
change parameter name of updateFocusState function
shivam-daksh Nov 20, 2024
021bf28
modify the shift+tab conditions
shivam-daksh Nov 20, 2024
8f2d982
fix the reverse dom order using shift+tab
shivam-daksh Nov 21, 2024
fdcb523
Merge branch 'learningequality:develop' into nav-order-ktable
shivam-daksh Nov 21, 2024
dae4c63
add comment to the change for readability
shivam-daksh Nov 21, 2024
46d6cd7
improve performance by replacing querySelectorAll
shivam-daksh Nov 22, 2024
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
223 changes: 107 additions & 116 deletions lib/KTable/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,156 +294,147 @@
* - header highlight
*/
handleKeydown(event, rowIndex, colIndex) {
const key = event.key;
switch (event.key) {
case 'ArrowUp':
case 'ArrowDown':
case 'ArrowLeft':
case 'ArrowRight':
this.handleArrowKeys(event.key, rowIndex, colIndex);
break;
case 'Enter':
this.handleEnterKey(rowIndex, colIndex);
break;
case 'Tab':
this.handleTabKey(event, rowIndex, colIndex);
break;
default:
break;
}
},

handleArrowKeys(key, rowIndex, colIndex) {
const totalRows = this.rows.length;
const totalCols = this.headers.length;

const lastRowIndex = totalRows - 1;
const lastColIndex = totalCols - 1;
let nextRowIndex = rowIndex;
let nextColIndex = colIndex;

switch (key) {
case 'ArrowUp':
if (rowIndex === -1) {
nextRowIndex = totalRows - 1;
} else {
nextRowIndex = rowIndex - 1;
}
nextRowIndex = rowIndex === -1 ? lastRowIndex : rowIndex - 1;
break;
case 'ArrowDown':
if (rowIndex === -1) {
nextRowIndex = 0;
} else if (rowIndex === totalRows - 1) {
nextRowIndex = -1;
} else {
nextRowIndex = (rowIndex + 1) % totalRows;
}
nextRowIndex = rowIndex === -1 ? 0 : rowIndex === lastRowIndex ? -1 : rowIndex + 1;
break;
case 'ArrowLeft':
if (rowIndex === -1) {
if (colIndex > 0) {
nextColIndex = colIndex - 1;
} else {
nextColIndex = totalCols - 1;
nextRowIndex = totalRows - 1;
}
} else if (colIndex > 0) {
nextColIndex = colIndex - 1;
nextColIndex = colIndex > 0 ? colIndex - 1 : lastColIndex;
nextRowIndex = colIndex === 0 ? lastRowIndex : -1;
} else {
nextColIndex = totalCols - 1;
nextRowIndex = rowIndex > 0 ? rowIndex - 1 : -1;
nextColIndex = colIndex > 0 ? colIndex - 1 : lastColIndex;
nextRowIndex = colIndex === 0 ? (rowIndex > 0 ? rowIndex - 1 : -1) : rowIndex;
}
break;
case 'ArrowRight':
if (colIndex === totalCols - 1) {
if (rowIndex === totalRows - 1) {
nextColIndex = 0;
nextRowIndex = -1;
} else {
nextColIndex = 0;
nextRowIndex = rowIndex + 1;
}
if (colIndex === lastColIndex) {
nextColIndex = 0;
nextRowIndex = rowIndex === lastRowIndex ? -1 : rowIndex + 1;
} else {
nextColIndex = colIndex + 1;
}
break;
case 'Enter':
if (rowIndex === -1 && this.sortable) {
this.handleSort(colIndex);
}
break;
case 'Tab': {
// Identify all focusable elements inside the current cell
const currentCell = this.getCell(rowIndex, colIndex);
}
this.updateFocusState(nextRowIndex, nextColIndex);
event.preventDefault();
},

// Collect focusable elements using native DOM methods
const focusableElements = [];
handleEnterKey(rowIndex, colIndex) {
if (rowIndex === -1 && this.sortable) {
this.handleSort(colIndex);
}
},

if (currentCell) {
const buttons = currentCell.getElementsByTagName('button');
const links = currentCell.getElementsByTagName('a');
const inputs = currentCell.getElementsByTagName('input');
const selects = currentCell.getElementsByTagName('select');
const textareas = currentCell.getElementsByTagName('textarea');
handleTabKey(event, rowIndex, colIndex) {
const totalRows = this.rows.length;
const totalCols = this.headers.length;
let nextRowIndex = rowIndex;
let nextColIndex = colIndex;

focusableElements.push(...buttons, ...links, ...inputs, ...selects, ...textareas);
}
const currentCell = this.getCell(rowIndex, colIndex);
const focusableElements = this.getFocusableElements(currentCell);
const focusedElement = document.activeElement;

const focusedElementIndex = focusableElements.indexOf(document.activeElement);
if (focusableElements.length > 0) {
if (!event.shiftKey) {
// if navigating between more focusable elements within the cell
if (focusedElementIndex < focusableElements.length - 1) {
focusableElements[focusedElementIndex + 1].focus();
event.preventDefault();
return;
} else {
if (colIndex < totalCols - 1) {
nextColIndex = colIndex + 1;
} else if (rowIndex < totalRows - 1) {
nextColIndex = 0;
nextRowIndex = rowIndex + 1;
} else {
// Allow default behavior when reaching the last cell
return;
}
}
} else {
if (focusedElementIndex < focusableElements.length - 1) {
// if navigating between more focusable elements within the cell
focusableElements[focusedElementIndex + 1].focus();
event.preventDefault();
return;
} else {
if (colIndex > 0) {
nextColIndex = colIndex - 1;
} else if (rowIndex > 0) {
nextColIndex = totalCols - 1;
nextRowIndex = rowIndex - 1;
} else {
// Allow default behavior when reaching the first cell
return;
}
}
}
// Include the cell itself as the first focusable element
const cellAndFocusableElements = [currentCell, ...focusableElements];
const focusedIndex = cellAndFocusableElements.indexOf(focusedElement);

if (!event.shiftKey) {
// Tab key navigation
if (focusedIndex < cellAndFocusableElements.length - 1) {
// Move to the next focusable element within the cell
cellAndFocusableElements[focusedIndex + 1].focus();
event.preventDefault();
return;
} else {
// Move to the first focusable element of the next cell
if (colIndex < totalCols - 1) {
nextColIndex = colIndex + 1;
} else if (rowIndex < totalRows - 1) {
nextColIndex = 0;
nextRowIndex = rowIndex + 1;
} else {
if (!event.shiftKey) {
if (colIndex < totalCols - 1) {
nextColIndex = colIndex + 1;
} else if (rowIndex < totalRows - 1) {
nextColIndex = 0;
nextRowIndex = rowIndex + 1;
} else {
// Allow default behavior when reaching the last cell
return;
}
} else {
if (colIndex > 0) {
nextColIndex = colIndex - 1;
} else if (rowIndex > 0) {
nextColIndex = totalCols - 1;
nextRowIndex = rowIndex - 1;
} else {
// Allow default behavior when reaching the first cell
return;
}
}
return; // Allow default Tab behavior when reaching the end
}

break;
const nextCell = this.getCell(nextRowIndex, nextColIndex);
const nextFocusableElements = this.getFocusableElements(nextCell);
const nextCellAndFocusableElements = [nextCell, ...nextFocusableElements];
nextCellAndFocusableElements[0].focus();
this.updateFocusState(nextRowIndex, nextColIndex);
event.preventDefault();
}

default:
} else {
// Shift+Tab key navigation
if (focusedIndex > 0) {
// Move to the previous focusable element within the cell
cellAndFocusableElements[focusedIndex - 1].focus();
event.preventDefault();
return;
} else {
// Move to the last focusable element of the previous cell
if (colIndex > 0) {
nextColIndex = colIndex - 1;
} else if (rowIndex > 0) {
nextColIndex = totalCols - 1;
nextRowIndex = rowIndex - 1;
} else {
return; // Allow default Shift+Tab behavior when at the beginning
}
const prevCell = this.getCell(nextRowIndex, nextColIndex);
const prevFocusableElements = this.getFocusableElements(prevCell);
const prevCellAndFocusableElements = [prevCell, ...prevFocusableElements];
prevCellAndFocusableElements[prevCellAndFocusableElements.length - 1].focus();
this.updateFocusState(nextRowIndex, nextColIndex, false);
event.preventDefault();
}
}

this.focusCell(nextRowIndex, nextColIndex);
},
updateFocusState(nextRowIndex, nextColIndex, shouldFocusCell = true) {
this.focusedRowIndex = nextRowIndex === -1 ? null : nextRowIndex;
this.focusedColIndex = nextColIndex;

this.highlightHeader(nextColIndex);
// Focus the cell only if it is necessary
if (shouldFocusCell) {
this.focusCell(nextRowIndex, nextColIndex);
}
},

event.preventDefault();
getFocusableElements(cell) {
if (!cell) return [];
const selectors = 'button, a, input, select, textarea';
return Array.from(cell.querySelectorAll(selectors));
Copy link
Contributor

Choose a reason for hiding this comment

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

About the use of querySelectorAll here, I was actually having a little lag in the navigation behaviour when moving through the focusable elements - this reminded me of an earlier code review comment here. We were using querySelectorAll earlier as well, but it caused performance issues, so we shifted to getElementByTagname and it had a very positive impact on the same. Can you update this code to make use of the previous getElementbyTagname logic? or maybe if there's any other conclusion you derive from that conversation about performance improvement- it is more than welcome. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BabyElias, Thanks for noticing. I've changed it to getElementsByTagName.

},

getCell(rowIndex, colIndex) {
if (rowIndex === -1) {
return this.$refs[`header-${colIndex}`][0];
Expand Down