Enhance thread navigation with improved expand/collapse behavior
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 <noreply@anthropic.com>
This commit is contained in:
12
CLAUDE.md
12
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
|
||||
|
14
README.md
14
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
|
||||
|
@ -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
|
||||
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
|
Reference in New Issue
Block a user