Optimizing keyboard navigation and selection handling

This commit is contained in:
2026-03-21 18:08:13 +01:00
parent 853bc4abae
commit f887267362
5 changed files with 82 additions and 25 deletions

View File

@@ -1,7 +1,15 @@
/** /**
* Create keyboard bindings * Create keyboard bindings
*/ */
// Set window.KEYBOARD_DEBUG = true in the browser console to enable traces
window.KEYBOARD_DEBUG = false;
(function () { (function () {
function kbLog(...args) {
if (window.KEYBOARD_DEBUG) console.debug('[Keyboard]', ...args);
}
/** /**
* Global registry to store keyboard shortcuts for multiple elements * Global registry to store keyboard shortcuts for multiple elements
*/ */
@@ -172,8 +180,11 @@
// Add snapshot to history // Add snapshot to history
KeyboardRegistry.snapshotHistory.push(snapshot); KeyboardRegistry.snapshotHistory.push(snapshot);
kbLog(`key="${key}" | history length=${KeyboardRegistry.snapshotHistory.length} | registeredElements=${KeyboardRegistry.elements.size}`);
// Cancel any pending timeout // Cancel any pending timeout
if (KeyboardRegistry.pendingTimeout) { if (KeyboardRegistry.pendingTimeout) {
kbLog(` cancelled pending timeout`);
clearTimeout(KeyboardRegistry.pendingTimeout); clearTimeout(KeyboardRegistry.pendingTimeout);
KeyboardRegistry.pendingTimeout = null; KeyboardRegistry.pendingTimeout = null;
KeyboardRegistry.pendingMatches = []; KeyboardRegistry.pendingMatches = [];
@@ -198,8 +209,7 @@
const currentNode = traverseTree(treeRoot, KeyboardRegistry.snapshotHistory); const currentNode = traverseTree(treeRoot, KeyboardRegistry.snapshotHistory);
if (!currentNode) { if (!currentNode) {
// No match in this tree, continue to next element kbLog(` element="${elementId}" → no match in tree`);
// console.debug("No match in tree for event", key);
continue; continue;
} }
@@ -212,6 +222,8 @@
// Check if there are longer sequences possible (node has children) // Check if there are longer sequences possible (node has children)
const hasLongerSequences = currentNode.children.size > 0; const hasLongerSequences = currentNode.children.size > 0;
kbLog(` element="${elementId}" | isInside=${isInside} | hasMatch=${hasMatch} | hasLongerSequences=${hasLongerSequences}`);
// Track if ANY element has longer sequences possible // Track if ANY element has longer sequences possible
if (hasLongerSequences) { if (hasLongerSequences) {
anyHasLongerSequence = true; anyHasLongerSequence = true;
@@ -221,6 +233,7 @@
if (hasMatch) { if (hasMatch) {
const requireInside = currentNode.config["require_inside"] === true; const requireInside = currentNode.config["require_inside"] === true;
const enabled = isCombinationEnabled(data.controlDivId, currentNode.combinationStr); const enabled = isCombinationEnabled(data.controlDivId, currentNode.combinationStr);
kbLog(` combination="${currentNode.combinationStr}" | requireInside=${requireInside} | enabled=${enabled}`);
if (enabled && (!requireInside || isInside)) { if (enabled && (!requireInside || isInside)) {
currentMatches.push({ currentMatches.push({
elementId: elementId, elementId: elementId,
@@ -228,10 +241,14 @@
combinationStr: currentNode.combinationStr, combinationStr: currentNode.combinationStr,
isInside: isInside isInside: isInside
}); });
} else {
kbLog(` → skipped (requireInside=${requireInside} but isInside=${isInside}, or disabled)`);
} }
} }
} }
kbLog(` result: matches=${currentMatches.length} | anyHasLongerSequence=${anyHasLongerSequence}`);
// Prevent default if we found any match and not in input context // Prevent default if we found any match and not in input context
if (currentMatches.length > 0 && !isInInputContext()) { if (currentMatches.length > 0 && !isInInputContext()) {
event.preventDefault(); event.preventDefault();
@@ -239,6 +256,7 @@
// Decision logic based on matches and longer sequences // Decision logic based on matches and longer sequences
if (currentMatches.length > 0 && !anyHasLongerSequence) { if (currentMatches.length > 0 && !anyHasLongerSequence) {
kbLog(` → TRIGGER immediately`);
// We have matches and NO element has longer sequences possible // We have matches and NO element has longer sequences possible
// Trigger ALL matches immediately // Trigger ALL matches immediately
for (const match of currentMatches) { for (const match of currentMatches) {
@@ -249,6 +267,7 @@
KeyboardRegistry.snapshotHistory = []; KeyboardRegistry.snapshotHistory = [];
} else if (currentMatches.length > 0 && anyHasLongerSequence) { } else if (currentMatches.length > 0 && anyHasLongerSequence) {
kbLog(` → WAITING ${KeyboardRegistry.sequenceTimeout}ms (longer sequence possible)`);
// We have matches but AT LEAST ONE element has longer sequences possible // We have matches but AT LEAST ONE element has longer sequences possible
// Wait for timeout - ALL current matches will be triggered if timeout expires // Wait for timeout - ALL current matches will be triggered if timeout expires
@@ -256,6 +275,7 @@
const savedEvent = event; // Save event for timeout callback const savedEvent = event; // Save event for timeout callback
KeyboardRegistry.pendingTimeout = setTimeout(() => { KeyboardRegistry.pendingTimeout = setTimeout(() => {
kbLog(` → TRIGGER after timeout`);
// Timeout expired, trigger ALL pending matches // Timeout expired, trigger ALL pending matches
for (const match of KeyboardRegistry.pendingMatches) { for (const match of KeyboardRegistry.pendingMatches) {
triggerHtmxAction(match.elementId, match.config, match.combinationStr, match.isInside, savedEvent); triggerHtmxAction(match.elementId, match.config, match.combinationStr, match.isInside, savedEvent);
@@ -268,10 +288,12 @@
}, KeyboardRegistry.sequenceTimeout); }, KeyboardRegistry.sequenceTimeout);
} else if (currentMatches.length === 0 && anyHasLongerSequence) { } else if (currentMatches.length === 0 && anyHasLongerSequence) {
kbLog(` → WAITING (partial match, no full match yet)`);
// No matches yet but longer sequences are possible // No matches yet but longer sequences are possible
// Just wait, don't trigger anything // Just wait, don't trigger anything
} else { } else {
kbLog(` → NO MATCH, clearing history`);
// No matches and no longer sequences possible // No matches and no longer sequences possible
// This is an invalid sequence - clear history // This is an invalid sequence - clear history
KeyboardRegistry.snapshotHistory = []; KeyboardRegistry.snapshotHistory = [];
@@ -280,11 +302,13 @@
// If we found no match at all, clear the history // If we found no match at all, clear the history
// This handles invalid sequences like "A C" when only "A B" exists // This handles invalid sequences like "A C" when only "A B" exists
if (!foundAnyMatch) { if (!foundAnyMatch) {
kbLog(` → foundAnyMatch=false, clearing history`);
KeyboardRegistry.snapshotHistory = []; KeyboardRegistry.snapshotHistory = [];
} }
// Also clear history if it gets too long (prevent memory issues) // Also clear history if it gets too long (prevent memory issues)
if (KeyboardRegistry.snapshotHistory.length > 10) { if (KeyboardRegistry.snapshotHistory.length > 10) {
kbLog(` → history too long, clearing`);
KeyboardRegistry.snapshotHistory = []; KeyboardRegistry.snapshotHistory = [];
} }
} }

View File

@@ -146,6 +146,15 @@
outline: none; outline: none;
} }
.dt2-cell-input {
width: 100%;
}
.dt2-cell-input:focus {
outline: none;
box-shadow: none;
}
.dt2-cell:hover, .dt2-cell:hover,
.dt2-selected-cell { .dt2-selected-cell {
background-color: var(--color-selection); background-color: var(--color-selection);

View File

@@ -698,7 +698,7 @@ function updateDatagridSelection(datagridId) {
if (cellElement) { if (cellElement) {
cellElement.classList.add('dt2-selected-focus'); cellElement.classList.add('dt2-selected-focus');
cellElement.style.userSelect = 'text'; cellElement.style.userSelect = 'text';
cellElement.focus({ preventScroll: true }); requestAnimationFrame(() => cellElement.focus({ preventScroll: false }));
hasFocusedCell = true; hasFocusedCell = true;
} }
} else if (selectionType === 'cell') { } else if (selectionType === 'cell') {
@@ -750,7 +750,7 @@ function updateDatagridSelection(datagridId) {
if (!hasFocusedCell) { if (!hasFocusedCell) {
const grid = document.getElementById(datagridId); const grid = document.getElementById(datagridId);
if (grid) grid.focus({ preventScroll: true }); if (grid) requestAnimationFrame(() => grid.focus({ preventScroll: true }));
} }
} }

View File

@@ -1,3 +1,4 @@
import bisect
import html import html
import logging import logging
import re import re
@@ -80,6 +81,7 @@ class DatagridState(DbObject):
self.selection: DatagridSelectionState = DatagridSelectionState() self.selection: DatagridSelectionState = DatagridSelectionState()
self.cell_formats: dict = {} self.cell_formats: dict = {}
self.table_format: list = [] self.table_format: list = []
self.ns_visible_indices: list[int] | None = None
class DatagridSettings(DbObject): class DatagridSettings(DbObject):
@@ -403,8 +405,12 @@ class DataGrid(MultipleInstance):
if col.visible and col.type != ColumnType.RowSelection_] if col.visible and col.type != ColumnType.RowSelection_]
def _get_visible_row_indices(self) -> list[int]: def _get_visible_row_indices(self) -> list[int]:
df = self._get_filtered_df() if self._state.ns_visible_indices is None:
return list(df.index) if df is not None else [] if self._df is None:
self._state.ns_visible_indices = []
else:
self._state.ns_visible_indices = list(self._apply_filter(self._df).index)
return self._state.ns_visible_indices
def _navigate(self, pos: tuple, direction: str) -> tuple: def _navigate(self, pos: tuple, direction: str) -> tuple:
col_pos, row_index = pos col_pos, row_index = pos
@@ -421,11 +427,11 @@ class DataGrid(MultipleInstance):
prev_cols = [c for c in navigable_cols if c < col_pos] prev_cols = [c for c in navigable_cols if c < col_pos]
return (prev_cols[-1], row_index) if prev_cols else pos return (prev_cols[-1], row_index) if prev_cols else pos
elif direction == "down": elif direction == "down":
next_rows = [r for r in visible_rows if r > row_index] next_pos = bisect.bisect_right(visible_rows, row_index)
return (col_pos, next_rows[0]) if next_rows else pos return (col_pos, visible_rows[next_pos]) if next_pos < len(visible_rows) else pos
elif direction == "up": elif direction == "up":
prev_rows = [r for r in visible_rows if r < row_index] prev_pos = bisect.bisect_left(visible_rows, row_index) - 1
return (col_pos, prev_rows[-1]) if prev_rows else pos return (col_pos, visible_rows[prev_pos]) if prev_pos >= 0 else pos
return pos return pos
@@ -439,9 +445,13 @@ class DataGrid(MultipleInstance):
return None return None
def _update_current_position(self, pos): def _update_current_position(self, pos, reset_selection: bool = False):
self._state.selection.last_selected = self._state.selection.selected self._state.selection.last_selected = self._state.selection.selected
self._state.selection.selected = pos self._state.selection.selected = pos
if reset_selection:
self._state.selection.extra_selected.clear()
self._state.edition.under_edition = None
self._state.save() self._state.save()
def _get_format_rules(self, col_pos, row_index, col_def): def _get_format_rules(self, col_pos, row_index, col_def):
@@ -508,14 +518,23 @@ class DataGrid(MultipleInstance):
self._columns.insert(0, DataGridRowSelectionColumnState()) self._columns.insert(0, DataGridRowSelectionColumnState())
def _enter_edition(self, pos): def _enter_edition(self, pos):
logger.debug(f"enter_edition: {pos=}")
col_pos, row_index = pos col_pos, row_index = pos
col_def = self._columns[col_pos] col_def = self._columns[col_pos]
if col_def.type in (ColumnType.RowSelection_, ColumnType.RowIndex, ColumnType.Formula): if col_def.type in (ColumnType.RowSelection_, ColumnType.RowIndex, ColumnType.Formula):
return self.render_partial() return self.render_partial()
if col_def.type == ColumnType.Bool:
return self._toggle_bool_cell(col_pos, row_index, col_def)
self._state.edition.under_edition = pos self._state.edition.under_edition = pos
self._state.save() self._state.save()
return self.render_partial("cell", pos=pos) return self.render_partial("cell", pos=pos)
def _toggle_bool_cell(self, col_pos, row_index, col_def):
col_array = self._fast_access.get(col_def.col_id)
current_value = col_array[row_index] if col_array is not None and row_index < len(col_array) else False
self._data_service.set_data(col_def.col_id, row_index, not bool(current_value))
return self.render_partial("cell", pos=(col_pos, row_index))
def _convert_edition_value(self, value_str, col_type): def _convert_edition_value(self, value_str, col_type):
if col_type == ColumnType.Number: if col_type == ColumnType.Number:
try: try:
@@ -645,11 +664,13 @@ class DataGrid(MultipleInstance):
def filter(self): def filter(self):
logger.debug("filter") logger.debug("filter")
self._state.filtered[FILTER_INPUT_CID] = self._datagrid_filter.get_query() self._state.filtered[FILTER_INPUT_CID] = self._datagrid_filter.get_query()
self._state.ns_visible_indices = None
return self.render_partial("body") return self.render_partial("body")
def handle_on_click(self, combination, is_inside, cell_id): def handle_on_click(self, combination, is_inside, cell_id):
logger.debug(f"on_click table={self.get_table_name()} {combination=} {is_inside=} {cell_id=}") logger.debug(f"on_click table={self.get_table_name()} {combination=} {is_inside=} {cell_id=}")
if is_inside and cell_id: if is_inside and cell_id:
logger.debug(f" is_inside=True")
self._state.selection.extra_selected.clear() self._state.selection.extra_selected.clear()
pos = self._get_pos_from_element_id(cell_id) pos = self._get_pos_from_element_id(cell_id)
@@ -659,9 +680,15 @@ class DataGrid(MultipleInstance):
pos == self._state.selection.selected and pos == self._state.selection.selected and
self._state.edition.under_edition is None): self._state.edition.under_edition is None):
return self._enter_edition(pos) return self._enter_edition(pos)
else:
logger.debug(
f" {pos=}, selected={self._state.selection.selected}, under_edition={self._state.edition.under_edition}")
self._update_current_position(pos) self._update_current_position(pos)
else:
logger.debug(f" is_inside=False")
return self.render_partial() return self.render_partial()
def on_mouse_selection(self, combination, is_inside, cell_id_mousedown, cell_id_mouseup): def on_mouse_selection(self, combination, is_inside, cell_id_mousedown, cell_id_mouseup):
@@ -685,13 +712,15 @@ class DataGrid(MultipleInstance):
def on_key_pressed(self, combination, has_focus, is_inside): def on_key_pressed(self, combination, has_focus, is_inside):
logger.debug(f"on_key_pressed table={self.get_table_name()} {combination=} {has_focus=} {is_inside=}") logger.debug(f"on_key_pressed table={self.get_table_name()} {combination=} {has_focus=} {is_inside=}")
if combination == "esc": if combination == "esc":
self._update_current_position(None) self._update_current_position(None, reset_selection=True)
self._state.selection.extra_selected.clear() return self.render_partial("cell", pos=self._state.selection.last_selected)
elif (combination == "enter" and elif (combination == "enter" and
self._settings.enable_edition and self._settings.enable_edition and
self._state.selection.selected and self._state.selection.selected and
self._state.edition.under_edition is None): self._state.edition.under_edition is None):
return self._enter_edition(self._state.selection.selected) return self._enter_edition(self._state.selection.selected)
elif combination in self._ARROW_KEY_DIRECTIONS: elif combination in self._ARROW_KEY_DIRECTIONS:
current_pos = (self._state.selection.selected current_pos = (self._state.selection.selected
or self._state.selection.last_selected or self._state.selection.last_selected
@@ -1139,11 +1168,6 @@ class DataGrid(MultipleInstance):
) )
def mk_selection_manager(self): def mk_selection_manager(self):
extra_attr = {
"hx-on::after-settle": f"updateDatagridSelection('{self._id}');",
}
selected = [] selected = []
if self._state.selection.selected: if self._state.selection.selected:
@@ -1157,7 +1181,6 @@ class DataGrid(MultipleInstance):
*[Div(selection_type=s_type, element_id=f"{elt_id}") for s_type, elt_id in selected], *[Div(selection_type=s_type, element_id=f"{elt_id}") for s_type, elt_id in selected],
id=f"tsm_{self._id}", id=f"tsm_{self._id}",
selection_mode=f"{self._state.selection.selection_mode}", selection_mode=f"{self._state.selection.selection_mode}",
**extra_attr,
) )
def mk_aggregation_cell(self, col_def, row_index: int, footer_conf, oob=False): def mk_aggregation_cell(self, col_def, row_index: int, footer_conf, oob=False):
@@ -1244,7 +1267,8 @@ class DataGrid(MultipleInstance):
id=self._id, id=self._id,
cls="grid", cls="grid",
style="height: 100%; grid-template-rows: auto 1fr;", style="height: 100%; grid-template-rows: auto 1fr;",
tabindex="-1" tabindex="-1",
**{"hx-on:htmx:after-swap": f"if(event.detail.target.id==='tsm_{self._id}') updateDatagridSelection('{self._id}');"}
) )
def render_partial(self, fragment="cell", **kwargs): def render_partial(self, fragment="cell", **kwargs):

View File

@@ -281,14 +281,14 @@ class TestDataGridBehaviour:
# Selection and Interaction # Selection and Interaction
# ------------------------------------------------------------------ # ------------------------------------------------------------------
def test_i_can_on_key_pressed_esc_clears_selection(self, datagrid): def test_i_can_on_key_pressed_esc_clears_selection(self, datagrid_with_data):
"""Test that pressing ESC resets both the focused cell and extra selections. """Test that pressing ESC resets both the focused cell and extra selections.
ESC is the standard 'deselect all' shortcut. Both selected and ESC is the standard 'deselect all' shortcut. Both selected and
extra_selected must be cleared so the grid visually deselects everything extra_selected must be cleared so the grid visually deselects everything
and subsequent navigation starts from a clean state. and subsequent navigation starts from a clean state.
""" """
dg = datagrid dg = datagrid_with_data
dg._state.selection.selected = (1, 2) dg._state.selection.selected = (1, 2)
dg._state.selection.extra_selected.append(("range", (0, 0, 2, 2))) dg._state.selection.extra_selected.append(("range", (0, 0, 2, 2)))