[PATCH yew-widget-toolkit 5/5] widget: data table: header: skip hidden cells in keyboard navigation

[Dominik Csapak <d.csapak@proxmox.com>]

When moving the header cursor with the arrow keys, we go over all
header cells, including the hidden ones. Since those are not rendered,
the focus does not move, so the user has to press the arrow key once
more for every hidden cell in between.

Also, only the cell of the cursor (or the first cell if there is no
cursor) gets 'tabindex=0'. When that cell is hidden, no rendered cell
is a tab stop and the header can't be reached with the tab key at all.
This happens e.g. when hiding the focused column via its own header
menu, or when the first header cell is hidden from the start, e.g. a
first column with 'hidden(true)'.

To reproduce:
* focus a column header and hide that column via its header menu
* try to tab into the header, it is skipped
* focus the column left of the hidden one and press the right arrow
  key, the focus does not move

Fix this by skipping hidden cells when moving the cursor, and by using
the first visible cell as tab stop when the cursor cell is hidden.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/widget/data_table/header_widget.rs | 57 +++++++++++++-------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/src/widget/data_table/header_widget.rs b/src/widget/data_table/header_widget.rs
index 2f55e88..0c0ff45 100644
--- a/src/widget/data_table/header_widget.rs
+++ b/src/widget/data_table/header_widget.rs
@@ -193,6 +193,21 @@ impl<T: 'static> PwtHeaderWidget<T> {
         span
     }
 
+    fn cell_visible(&self, cell_idx: usize) -> bool {
+        cell_idx < self.state.cell_count() && !self.state.get_cell_hidden(cell_idx)
+    }
+
+    // The cell that gets focus when tabbing into the header. Hidden cells are not rendered, so
+    // fall back to the first visible one.
+    fn tab_stop(&self) -> Option<usize> {
+        match self.cursor {
+            Some(cursor) if self.cell_visible(cursor) => Some(cursor),
+            Some(_) | None => {
+                (0..self.state.cell_count()).find(|cell_idx| self.cell_visible(*cell_idx))
+            }
+        }
+    }
+
     fn unique_cell_id(&self, cell_idx: usize) -> String {
         format!("{}-cell-{}", self.unique_id, cell_idx)
     }
@@ -208,11 +223,7 @@ impl<T: 'static> PwtHeaderWidget<T> {
     ) {
         let column_idx = cell.start_col;
         let cell_idx = cell.cell_idx;
-        let active = self
-            .cursor
-            .map(|cursor| cursor == cell_idx)
-            .unwrap_or(false);
-        let tabindex = if active || (self.cursor.is_none() && (cell_idx == 0)) {
+        let tabindex = if self.tab_stop() == Some(cell_idx) {
             AttrValue::Static("0")
         } else {
             AttrValue::Static("-1")
@@ -387,11 +398,7 @@ impl<T: 'static> PwtHeaderWidget<T> {
         header_row: &mut Vec<Html>,
     ) -> usize {
         let cell_idx = group.cell_idx;
-        let active = self
-            .cursor
-            .map(|cursor| cursor == cell_idx)
-            .unwrap_or(false);
-        let tabindex = if active || (self.cursor.is_none() && (cell_idx == 0)) {
+        let tabindex = if self.tab_stop() == Some(cell_idx) {
             "0"
         } else {
             "-1"
@@ -593,28 +600,22 @@ impl<T: 'static> Component for PwtHeaderWidget<T> {
                 true
             }
             Msg::MoveCursor(direction) => {
-                let last = self.state.cell_count().saturating_sub(1);
+                let count = self.state.cell_count();
                 let cursor = match self.cursor {
                     Some(cursor) => cursor,
                     None => return false,
                 };
-                self.cursor = Some(match direction {
-                    false => {
-                        if cursor > 0 {
-                            cursor - 1
-                        } else {
-                            last
-                        }
-                    }
-                    true => {
-                        if (cursor + 1) <= last {
-                            cursor + 1
-                        } else {
-                            0
-                        }
-                    }
-                });
-                self.focus_active_cell();
+                // Wrap around at either end and skip hidden cells, as they are not rendered.
+                let next = (1..count)
+                    .map(|step| match direction {
+                        true => (cursor + step) % count,
+                        false => (cursor + count - step) % count,
+                    })
+                    .find(|cell_idx| self.cell_visible(*cell_idx));
+                if let Some(next) = next {
+                    self.cursor = Some(next);
+                    self.focus_active_cell();
+                }
                 true
             }
         }
-- 
2.47.3