From 1391d0c66f29093edcc0c306e68f8133a24f7990 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Mon, 21 Jul 2025 10:07:41 -0400 Subject: [PATCH] Enhance thread navigation with improved expand/collapse behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Major improvements to threaded conversation navigation: - Add Shift+Left shortcut to navigate to thread root from any reply - Fix navigation state synchronization using accessibility flags instead of Qt's isExpanded() - Implement proper child item flag management (selectable/non-selectable) - Add comprehensive scrollToItem() calls for better focus management - Enhance navigation methods with safety checks and proper state handling - Force visual refresh attempts to work around Qt display sync issues Navigation improvements: - Right Arrow: Expand thread or move to first child when expanded - Left Arrow: Collapse thread or move to parent - Shift+Left: Jump to thread root from anywhere in the thread - All navigation properly skips collapsed items Known Qt quirk documented: Visual display may occasionally require double-collapse cycle due to Qt tree widget display synchronization, but navigation logic works correctly after first collapse. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- CLAUDE.md | 12 ++- README.md | 14 +++ src/accessibility/accessible_tree.py | 150 +++++++++++++++++++-------- 3 files changed, 129 insertions(+), 47 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d26a3cf..4b8c214 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -185,13 +185,21 @@ Timeline Item: "Alice posted: Hello world (3 replies, collapsed)" ``` **Key Behaviors:** -- Right Arrow: Expand thread, announce "expanded" -- Left Arrow: Collapse thread, announce "collapsed" +- Right Arrow: Expand thread, announce "expanded" / Move to first child when expanded +- Left Arrow: Collapse thread, announce "collapsed" / Move to parent +- Shift+Left Arrow: Navigate to thread root from any reply - Down Arrow: Next item (skip collapsed children) - Up Arrow: Previous item - Page Down/Up: Jump 5 items - Home/End: First/last item +### Known Qt Tree Widget Display Quirk + +Due to Qt's visual display synchronization, thread collapse may require double-operation: +- Navigation logic works correctly after first collapse (skips collapsed items) +- Visual display may need expand→collapse cycle to fully sync +- Workaround: Navigate to root with Shift+Left, then Left→Right→Left if needed + ### AccessibleTreeWidget Requirements - Inherit from QTreeWidget - Override keyPressEvent for custom navigation diff --git a/README.md b/README.md index 3fc3f19..d2a814b 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,9 @@ Bifrost includes a sophisticated sound system with: ### Navigation - **Arrow Keys**: Navigate through posts +- **Right Arrow**: Expand thread / Move to first child when expanded +- **Left Arrow**: Collapse thread / Move to parent +- **Shift+Left Arrow**: Navigate to thread root from any reply - **Page Up/Down**: Jump multiple posts - **Home/End**: Go to first/last post - **Enter**: Expand/collapse threads, or vote in polls @@ -156,6 +159,17 @@ Bifrost includes comprehensive poll support with full accessibility: - Thread expansion/collapse with audio feedback - Poll creation and voting with full accessibility support +### Known Qt Display Quirk + +Due to a Qt tree widget display synchronization issue, thread collapse may occasionally require a double-operation for the visual display to fully sync: + +**If collapse appears incomplete:** +1. Navigate to thread root post (use Shift+Left from any reply) +2. Press Left to collapse → announces "collapsed" +3. Press Right then Left again → now fully collapsed visually + +Note: Navigation logic works correctly after the first collapse (arrow keys will skip collapsed replies), but the visual display may need the extra cycle to sync. + ## Sound Pack Creation and Installation ### Creating Custom Sound Packs diff --git a/src/accessibility/accessible_tree.py b/src/accessibility/accessible_tree.py index 9d2fb54..07d983f 100644 --- a/src/accessibility/accessible_tree.py +++ b/src/accessibility/accessible_tree.py @@ -70,44 +70,55 @@ class AccessibleTreeWidget(QTreeWidget): if key == Qt.Key_Right: # Right Arrow (with or without Shift): Expand thread - if current.childCount() > 0 and not current.isExpanded(): - # Use Qt's built-in expand method - self.expandItem(current) - # Keep focus on the same item - self.setCurrentItem(current) - self.scrollToItem(current) # Prevent jumping to bottom - self.announce_item_state(current, "expanded") - return - # If already expanded or no children, move to first child (only without Shift) - elif current.childCount() > 0 and current.isExpanded() and not has_shift: - first_child = current.child(0) - if first_child: - self.setCurrentItem(first_child) - self.scrollToItem(first_child) + if current.childCount() > 0: + if not current.isExpanded(): + # Use Qt's built-in expand method - it will trigger on_item_expanded + self.expandItem(current) + # Force immediate update to ensure state synchronization + self.update_child_accessibility(current, True) return + # If already expanded and no shift, move to first child + elif not has_shift: + first_child = current.child(0) + if first_child and self.is_item_navigable(first_child): + self.setCurrentItem(first_child) + self.scrollToItem(first_child) + return elif key == Qt.Key_Left: - # Shift+Left or plain Left: Collapse thread if expanded - if current.childCount() > 0 and current.isExpanded(): - # Try direct setExpanded(False) instead of collapseItem() - current.setExpanded(False) - # Keep focus on the same item - self.setCurrentItem(current) - self.scrollToItem(current) - self.announce_item_state(current, "collapsed") - return - # Plain Left only: move to parent if already collapsed - elif current.parent() and not has_shift: - parent_item = current.parent() - self.setCurrentItem(parent_item) - self.scrollToItem(parent_item) # Ensure parent stays visible - return + if has_shift: + # Shift+Left: Just navigate to root thread parent + # (user can collapse manually from there for reliable behavior) + root_parent = self.get_root_parent(current) + if root_parent and root_parent != current: + # Move focus to the root parent + self.setCurrentItem(root_parent) + self.scrollToItem(root_parent) + return + else: + # Plain Left: Collapse current item if expanded, otherwise move to parent + if current.childCount() > 0 and current.isExpanded(): + # Collapse current item - this will trigger on_item_collapsed + self.collapseItem(current) + # Force immediate update to ensure state synchronization + self.update_child_accessibility(current, False) + # Force visual refresh to sync display with logical state + self.repaint() + self.update() + return + elif current.parent(): + # Move to immediate parent + parent_item = current.parent() + self.setCurrentItem(parent_item) + self.scrollToItem(parent_item) + return elif key == Qt.Key_Down: # Move to next visible item (skip collapsed children) next_item = self.get_next_visible_item(current) if next_item: self.setCurrentItem(next_item) + self.scrollToItem(next_item) return elif key == Qt.Key_Up: @@ -115,6 +126,7 @@ class AccessibleTreeWidget(QTreeWidget): prev_item = self.get_previous_visible_item(current) if prev_item: self.setCurrentItem(prev_item) + self.scrollToItem(prev_item) return elif key == Qt.Key_PageDown: @@ -127,6 +139,7 @@ class AccessibleTreeWidget(QTreeWidget): else: break self.setCurrentItem(target) + self.scrollToItem(target) return elif key == Qt.Key_PageUp: @@ -139,6 +152,7 @@ class AccessibleTreeWidget(QTreeWidget): else: break self.setCurrentItem(target) + self.scrollToItem(target) return elif key == Qt.Key_Home: @@ -146,6 +160,7 @@ class AccessibleTreeWidget(QTreeWidget): first_item = self.topLevelItem(0) if first_item: self.setCurrentItem(first_item) + self.scrollToItem(first_item) return elif key == Qt.Key_End: @@ -153,6 +168,7 @@ class AccessibleTreeWidget(QTreeWidget): last_item = self.get_last_visible_item() if last_item: self.setCurrentItem(last_item) + self.scrollToItem(last_item) return # Only fall back to default behavior if we didn't handle the key @@ -162,9 +178,14 @@ class AccessibleTreeWidget(QTreeWidget): def get_next_visible_item(self, item: QTreeWidgetItem) -> QTreeWidgetItem: """Get the next visible item in the tree""" - # If item has children and is expanded, go to first child - if item.childCount() > 0 and item.isExpanded(): - return item.child(0) + if not item: + return None + + # If item has children, check if first child is navigable (handles expansion state) + if item.childCount() > 0: + first_child = item.child(0) + if first_child and self.is_item_navigable(first_child): + return first_child # Otherwise, find next sibling or ancestor's sibling current = item @@ -174,20 +195,27 @@ class AccessibleTreeWidget(QTreeWidget): # Find next sibling index = parent.indexOfChild(current) if index + 1 < parent.childCount(): - return parent.child(index + 1) + next_sibling = parent.child(index + 1) + if next_sibling and self.is_item_navigable(next_sibling): + return next_sibling # No more siblings, go up to parent current = parent else: # Top-level item, find next top-level item index = self.indexOfTopLevelItem(current) if index + 1 < self.topLevelItemCount(): - return self.topLevelItem(index + 1) + next_item = self.topLevelItem(index + 1) + if next_item and self.is_item_navigable(next_item): + return next_item # No more items return None return None def get_previous_visible_item(self, item: QTreeWidgetItem) -> QTreeWidgetItem: """Get the previous visible item in the tree""" + if not item: + return None + parent = item.parent() if parent: @@ -196,25 +224,34 @@ class AccessibleTreeWidget(QTreeWidget): if index > 0: # Get previous sibling and its last visible descendant prev_sibling = parent.child(index - 1) - return self.get_last_visible_descendant(prev_sibling) + if prev_sibling and self.is_item_navigable(prev_sibling): + return self.get_last_visible_descendant(prev_sibling) else: # No previous sibling, go to parent - return parent + if self.is_item_navigable(parent): + return parent else: # Top-level item index = self.indexOfTopLevelItem(item) if index > 0: # Get previous top-level item and its last visible descendant prev_item = self.topLevelItem(index - 1) - return self.get_last_visible_descendant(prev_item) + if prev_item and self.is_item_navigable(prev_item): + return self.get_last_visible_descendant(prev_item) return None def get_last_visible_descendant(self, item: QTreeWidgetItem) -> QTreeWidgetItem: """Get the last visible descendant of an item""" - if item.childCount() > 0 and item.isExpanded(): - # Get the last child and its last visible descendant + if not item or not self.is_item_navigable(item): + return None + + # Check if any children are navigable (handles expansion state via accessibility) + if item.childCount() > 0: + # Get the last child and check if it's navigable last_child = item.child(item.childCount() - 1) - return self.get_last_visible_descendant(last_child) + if last_child and self.is_item_navigable(last_child): + result = self.get_last_visible_descendant(last_child) + return result if result else item return item def get_last_visible_item(self) -> QTreeWidgetItem: @@ -241,13 +278,18 @@ class AccessibleTreeWidget(QTreeWidget): for i in range(item.childCount()): child = item.child(i) if visible: - # Make child accessible - child.setFlags(child.flags() | Qt.ItemIsEnabled) + # Make child accessible and ensure proper selection + child.setFlags(child.flags() | Qt.ItemIsEnabled | Qt.ItemIsSelectable) child.setData(0, Qt.AccessibleDescriptionRole, "") # Clear hidden marker + # Recursively handle nested children + self.update_child_accessibility(child, child.isExpanded()) else: - # Hide from screen readers but keep in tree + # Hide from screen readers and navigation child.setData(0, Qt.AccessibleDescriptionRole, "hidden") - # Don't disable completely as it affects navigation + # Remove selectable flag - collapsed items should not be navigable + child.setFlags((child.flags() | Qt.ItemIsEnabled) & ~Qt.ItemIsSelectable) + # Hide all nested children too + self.update_child_accessibility(child, False) def announce_item_state(self, item: QTreeWidgetItem, state: str): """Announce item state change for screen readers""" @@ -291,4 +333,22 @@ class AccessibleTreeWidget(QTreeWidget): while current.parent(): depth += 1 current = current.parent() - return depth \ No newline at end of file + return depth + + def get_root_parent(self, item: QTreeWidgetItem) -> QTreeWidgetItem: + """Get the root parent (top-level item) of any item in a thread""" + current = item + while current.parent(): + current = current.parent() + return current + + def is_item_navigable(self, item: QTreeWidgetItem) -> bool: + """Check if an item is safe to navigate to""" + if not item: + return False + # Check if item is enabled and selectable, and not hidden + flags = item.flags() + is_hidden = item.data(0, Qt.AccessibleDescriptionRole) == "hidden" + is_enabled = bool(flags & Qt.ItemIsEnabled) + is_selectable = bool(flags & Qt.ItemIsSelectable) + return is_enabled and is_selectable and not is_hidden \ No newline at end of file