Skip to content

Try to fix QTreeWidget accessibility bug #3573

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

Draft
ann0see wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Try to fix QTreeWidget accessibility bug #3573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d148bc1
Try to fix QTreeWidget accessibility bug
ann0see Dec 6, 2025
46c45a3
Add bare bones, AI generated accessibility button
ann0see Dec 6, 2025
867e87c
Clean up probably invalid text
ann0see Dec 6, 2025
6101375
Fix bug
ann0see Dec 6, 2025
e6d543a
Revert "Fix bug"
ann0see Dec 6, 2025
1d39c1d
Revert "Clean up probably invalid text"
ann0see Dec 6, 2025
2ba71f7
Simplify text
ann0see Dec 6, 2025
0be5b53
Move around layout for more intuitive screen reader use
ann0see Dec 6, 2025
8f7929f
Remove ifdefs
ann0see Dec 19, 2025
0247b60
Small cleanups
ann0see Dec 19, 2025
4a08bfd
Run clang-format
ann0see Dec 25, 2025
6f43c78
Revert wrong spacing introduced by clang-format
ann0see Dec 25, 2025
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
274 changes: 274 additions & 0 deletions src/connectdlg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,89 @@ CConnectDlg::CConnectDlg ( CClientSettings* pNSetP, const bool bNewShowCompleteR
// per default the root shall not be decorated (to save space)
lvwServers->setRootIsDecorated ( false );

// Create simplified accessible navigation panel for screen readers
// Design: One info label + 2 navigation buttons (Previous/Next) + Toggle button

wAccessibleNavPanel = new QWidget ( this );
wAccessibleNavPanel->setObjectName ( "wAccessibleNavPanel" );

QVBoxLayout* accessibleMainLayout = new QVBoxLayout ( wAccessibleNavPanel );
accessibleMainLayout->setContentsMargins ( 0, 5, 0, 5 );

// Create horizontal layout for navigation buttons (Previous, Next)
QHBoxLayout* navLayout = new QHBoxLayout();

butAccessiblePrevious = new QPushButton ( tr ( "Previous Entry" ), wAccessibleNavPanel );
butAccessiblePrevious->setObjectName ( "butAccessiblePrevious" );
butAccessiblePrevious->setAccessibleName ( tr ( "Previous Button" ) );
butAccessiblePrevious->setAccessibleDescription ( tr ( "Navigate to the previous server list entry" ) );
butAccessiblePrevious->setShortcut ( QKeySequence ( Qt::ALT | Qt::Key_Up ) );
butAccessiblePrevious->setToolTip ( tr ( "Previous entry (Alt+Up)" ) );
navLayout->addWidget ( butAccessiblePrevious, 1 );

butAccessibleNext = new QPushButton ( tr ( "Next entry" ), wAccessibleNavPanel );
butAccessibleNext->setObjectName ( "butAccessibleNext" );
butAccessibleNext->setAccessibleName ( tr ( "Go to next entry" ) );
butAccessibleNext->setAccessibleDescription ( tr ( "Navigate to the next server list entry" ) );
butAccessibleNext->setShortcut ( QKeySequence ( Qt::ALT | Qt::Key_Down ) );
butAccessibleNext->setToolTip ( tr ( "Next entry (Alt+Down)" ) );
navLayout->addWidget ( butAccessibleNext, 1 );

accessibleMainLayout->addLayout ( navLayout );

// Create read-only, focusable label showing current server information
// Using QLabel with focus policy so screenreaders can read it
lblAccessibleServerInfo = new QLabel ( tr ( "No entry selected" ), wAccessibleNavPanel );
lblAccessibleServerInfo->setObjectName ( "lblAccessibleServerInfo" );
lblAccessibleServerInfo->setWordWrap ( true );
lblAccessibleServerInfo->setFrameStyle ( QFrame::Panel | QFrame::Sunken );
lblAccessibleServerInfo->setTextInteractionFlags ( Qt::TextSelectableByMouse | Qt::TextSelectableByKeyboard );
lblAccessibleServerInfo->setMinimumHeight ( 50 );
lblAccessibleServerInfo->setFocusPolicy ( Qt::StrongFocus ); // Make it focusable for screen readers
lblAccessibleServerInfo->setAccessibleName ( tr ( "Current server information" ) );
lblAccessibleServerInfo->setAccessibleDescription (
tr ( "Shows details of the currently selected server. Use Previous/Next buttons or Alt+Up/Down to navigate." ) );
accessibleMainLayout->addWidget ( lblAccessibleServerInfo );

// Create toggle button
butToggleAccessible = new QPushButton ( tr ( "Hide Accessible Controls" ), this );
butToggleAccessible->setObjectName ( "butToggleAccessible" );
butToggleAccessible->setAccessibleName ( tr ( "Toggle accessible controls" ) );
butToggleAccessible->setAccessibleDescription ( tr ( "Show or hide the accessible navigation panel for screen readers" ) );
butToggleAccessible->setCheckable ( true );
butToggleAccessible->setChecked ( true );
butToggleAccessible->setToolTip ( tr ( "Toggle accessible controls" ) );

// Insert the accessible panel and toggle button into the layout right after the tree widget
QVBoxLayout* mainLayout = qobject_cast<QVBoxLayout*> ( layout() );
if ( mainLayout )
{
// Find the tree widget in the layout
bool inserted = false;
for ( int i = 0; i < mainLayout->count(); ++i )
{
QLayoutItem* item = mainLayout->itemAt ( i );
if ( item && item->widget() == lvwServers )
{
mainLayout->insertWidget ( i + 1, butToggleAccessible );
mainLayout->insertWidget ( i + 2, wAccessibleNavPanel );
inserted = true;
break;
}
}
if ( !inserted )
{
qWarning ( "Accessible navigation panel could not be inserted: tree widget not found in layout." );
}
}
else
{
qWarning ( "Accessible navigation panel could not be inserted: main layout cast failed." );
}

// Initially show the accessible panel
wAccessibleNavPanel->setVisible ( true );
Copy link
Collaborator

@pljones pljones Dec 6, 2025

Choose a reason for hiding this comment

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

Using the setting, you'd set to false immediate after creating and based on the setting here.


// make sure the connect button has the focus
butConnect->setFocus();

Expand Down Expand Up @@ -243,6 +326,9 @@ CConnectDlg::CConnectDlg ( CClientSettings* pNSetP, const bool bNewShowCompleteR
// to get default return key behaviour working
QObject::connect ( lvwServers, &QTreeWidget::activated, this, &CConnectDlg::OnConnectClicked );

// connect selection change for accessibility support
QObject::connect ( lvwServers, &QTreeWidget::itemSelectionChanged, this, &CConnectDlg::OnServerListItemSelectionChanged );

// line edit
QObject::connect ( edtFilter, &QLineEdit::textEdited, this, &CConnectDlg::OnFilterTextEdited );

Expand All @@ -266,6 +352,11 @@ CConnectDlg::CConnectDlg ( CClientSettings* pNSetP, const bool bNewShowCompleteR
QObject::connect ( &TimerPing, &QTimer::timeout, this, &CConnectDlg::OnTimerPing );

QObject::connect ( &TimerReRequestServList, &QTimer::timeout, this, &CConnectDlg::OnTimerReRequestServList );

// accessible navigation panel
QObject::connect ( butAccessiblePrevious, &QPushButton::clicked, this, &CConnectDlg::OnAccessiblePreviousClicked );
QObject::connect ( butAccessibleNext, &QPushButton::clicked, this, &CConnectDlg::OnAccessibleNextClicked );
QObject::connect ( butToggleAccessible, &QPushButton::clicked, this, &CConnectDlg::OnToggleAccessibleClicked );
}

void CConnectDlg::showEvent ( QShowEvent* )
Expand Down Expand Up @@ -631,6 +722,189 @@ void CConnectDlg::OnServerAddrEditTextChanged ( const QString& )
lvwServers->clearSelection();
}

void CConnectDlg::OnServerListItemSelectionChanged() { UpdateAccessibleServerInfo(); }

void CConnectDlg::UpdateAccessibleServerInfo()
Copy link
Collaborator

@pljones pljones Dec 7, 2025

Choose a reason for hiding this comment

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

Needs tests showing:

  1. Empty filter, Show all musicians off
Image 2. Empty filter, Show all musicians on Image 3. Occupied servers filter on, Show all musicians off Image 3. Occupied servers filter on, Show all musicians on Image

{
QList<QTreeWidgetItem*> selectedItems = lvwServers->selectedItems();

if ( !selectedItems.isEmpty() && selectedItems.first()->parent() == nullptr )
Copy link
Collaborator

@pljones pljones Dec 7, 2025

Choose a reason for hiding this comment

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

Need to comment this to explain it has to handle Server entries and Client entries separately.

{
// We have a server item selected (not a musician child item)
QTreeWidgetItem* pItem = selectedItems.first();

// Extract server information
QString serverName = pItem->text ( LVC_NAME );
QString pingTime = pItem->text ( LVC_PING );
QString musicians = pItem->text ( LVC_CLIENTS );
QString location = pItem->text ( LVC_LOCATION );
QString version = pItem->text ( LVC_VERSION );

// Build text for screen readers
QString accessibleText = tr ( "Server: %1" ).arg ( serverName );
if ( !pingTime.isEmpty() )
{
accessibleText += tr ( ", Ping: %1" ).arg ( pingTime );
}
if ( !musicians.isEmpty() )
{
accessibleText += tr ( ", Musicians: %1" ).arg ( musicians );
}
if ( !location.isEmpty() )
{
accessibleText += tr ( ", Location: %1" ).arg ( location );
}
if ( !version.isEmpty() )
{
accessibleText += tr ( ", Version: %1" ).arg ( version );
}

// Update label
lblAccessibleServerInfo->setText ( accessibleText );
lblAccessibleServerInfo->setAccessibleName ( tr ( "Selected server information" ) );
lblAccessibleServerInfo->setAccessibleDescription ( accessibleText );

// Update navigation buttons to show previous/next server names
int currentIndex = lvwServers->indexOfTopLevelItem ( pItem );

// Update "Previous" button with previous server name
if ( currentIndex > 0 )
{
QTreeWidgetItem* prevItem = lvwServers->topLevelItem ( currentIndex - 1 );
QString prevName = prevItem->text ( LVC_NAME );
butAccessiblePrevious->setText ( prevName );
butAccessiblePrevious->setAccessibleName ( tr ( "Previous server: %1" ).arg ( prevName ) );
butAccessiblePrevious->setAccessibleDescription ( tr ( "Go to previous server: %1" ).arg ( prevName ) );
butAccessiblePrevious->setEnabled ( true );
}
else
{
butAccessiblePrevious->setText ( tr ( "(first)" ) );
butAccessiblePrevious->setAccessibleName ( tr ( "No previous server - at first server" ) );
butAccessiblePrevious->setAccessibleDescription ( tr ( "Cannot go back, already at first server" ) );
butAccessiblePrevious->setEnabled ( false );
}

// Update "Next" button with next server name
if ( currentIndex < lvwServers->topLevelItemCount() - 1 )
{
QTreeWidgetItem* nextItem = lvwServers->topLevelItem ( currentIndex + 1 );
QString nextName = nextItem->text ( LVC_NAME );
butAccessibleNext->setText ( nextName );
butAccessibleNext->setAccessibleName ( tr ( "Next server: %1" ).arg ( nextName ) );
butAccessibleNext->setAccessibleDescription ( tr ( "Go to next server: %1" ).arg ( nextName ) );
butAccessibleNext->setEnabled ( true );
}
else
{
butAccessibleNext->setText ( tr ( "(last)" ) );
butAccessibleNext->setAccessibleName ( tr ( "No next server - at last server" ) );
butAccessibleNext->setAccessibleDescription ( tr ( "Cannot go forward, already at last server" ) );
butAccessibleNext->setEnabled ( false );
}

// Force VoiceOver to announce the change
QAccessible::updateAccessibility ( new QAccessibleValueChangeEvent ( lblAccessibleServerInfo, accessibleText ) );
}
else
{
// Reset navigation buttons
butAccessiblePrevious->setText ( tr ( "Previous" ) );
butAccessiblePrevious->setAccessibleName ( tr ( "Navigate to previous server" ) );
butAccessiblePrevious->setEnabled ( lvwServers->topLevelItemCount() > 0 );

butAccessibleNext->setText ( tr ( "Next" ) );
butAccessibleNext->setAccessibleName ( tr ( "Navigate to next server" ) );
butAccessibleNext->setEnabled ( lvwServers->topLevelItemCount() > 0 );

// No server selected or musician child selected
lblAccessibleServerInfo->setText ( tr ( "<i>No server selected or in musician selection</i>" ) );
lblAccessibleServerInfo->setAccessibleName ( tr ( "No server selected or in musician selection" ) );
lblAccessibleServerInfo->setAccessibleDescription (
tr ( "No server selected. Use Previous/Next buttons or Alt+Up/Down to navigate servers." ) );
}
}

void CConnectDlg::OnAccessiblePreviousClicked()
Copy link
Collaborator

@pljones pljones Dec 7, 2025

Choose a reason for hiding this comment

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

So these will trigger UpdateAccessibleServerInfo through

QObject::connect ( lvwServers, &QTreeWidget::itemSelectionChanged, this, &CConnectDlg::OnServerListItemSelectionChanged );

Copy link
Collaborator

@pljones pljones Dec 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think they should skip Clients -- it needs to step through all entries in the list, otherwise the list isn't fully accessible.

{
// Navigate to previous server
QList<QTreeWidgetItem*> selectedItems = lvwServers->selectedItems();
QTreeWidgetItem* currentItem = selectedItems.isEmpty() ? nullptr : selectedItems.first();

// Get current item or first item if none selected
if ( currentItem == nullptr )
{
// Select first item
if ( lvwServers->topLevelItemCount() > 0 )
{
lvwServers->setCurrentItem ( lvwServers->topLevelItem ( 0 ) );
}
return;
}

// If current item is a musician (child), move to parent
if ( currentItem->parent() != nullptr )
{
lvwServers->setCurrentItem ( currentItem->parent() );
return;
}

// Get previous server item
int currentIndex = lvwServers->indexOfTopLevelItem ( currentItem );
if ( currentIndex > 0 )
{
lvwServers->setCurrentItem ( lvwServers->topLevelItem ( currentIndex - 1 ) );
}
}

void CConnectDlg::OnAccessibleNextClicked()
{
// Navigate to next server
QList<QTreeWidgetItem*> selectedItems = lvwServers->selectedItems();
QTreeWidgetItem* currentItem = selectedItems.isEmpty() ? nullptr : selectedItems.first();

// Get current item or first item if none selected
if ( currentItem == nullptr )
{
// Select first item
if ( lvwServers->topLevelItemCount() > 0 )
{
lvwServers->setCurrentItem ( lvwServers->topLevelItem ( 0 ) );
}
return;
}

// If current item is a musician (child), find next server
if ( currentItem->parent() != nullptr )
{
currentItem = currentItem->parent();
}

// Find next server item (skip musician children)
int currentIndex = lvwServers->indexOfTopLevelItem ( currentItem );
if ( currentIndex >= 0 && currentIndex < lvwServers->topLevelItemCount() - 1 )
{
lvwServers->setCurrentItem ( lvwServers->topLevelItem ( currentIndex + 1 ) );
}
}

void CConnectDlg::OnToggleAccessibleClicked()
{
bool isVisible = wAccessibleNavPanel->isVisible();
wAccessibleNavPanel->setVisible ( !isVisible );

if ( isVisible )
{
butToggleAccessible->setText ( u8"\u25B6 " + tr ( "Show Accessible Controls" ) );
butToggleAccessible->setAccessibleDescription ( tr ( "Show the accessible navigation controls" ) );
}
else
{
butToggleAccessible->setText ( u8"\u25BC " + tr ( "Hide Accessible Controls" ) );
butToggleAccessible->setAccessibleDescription ( tr ( "Hide the accessible navigation controls" ) );
}
}

void CConnectDlg::OnCustomDirectoriesChanged()
{

Expand Down
15 changes: 15 additions & 0 deletions src/connectdlg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#include <QLocale>
#include <QtConcurrent>
#include <QRegularExpression>
#include <QAccessible>
#include <QHBoxLayout>
#include <QVBoxLayout>
#include "global.h"
#include "util.h"
#include "settings.h"
Expand Down Expand Up @@ -104,9 +107,17 @@ class CConnectDlg : public CBaseDlg, private Ui_CConnectDlgBase
void RequestServerList();
void EmitCLServerListPingMes ( const CHostAddress& haServerAddress, const bool bNeedVersion );
void UpdateDirectoryComboBox();
void UpdateAccessibleServerInfo();

CClientSettings* pSettings;

// Accessible navigation panel for screen readers
QWidget* wAccessibleNavPanel; // Container for accessible navigation
QLabel* lblAccessibleServerInfo; // Label showing current server information (read-only)
QPushButton* butAccessiblePrevious; // Button to navigate to previous server
QPushButton* butAccessibleNext; // Button to navigate to next server
QPushButton* butToggleAccessible; // Button to show/hide accessible panel

QTimer TimerPing;
QTimer TimerReRequestServList;
QTimer TimerInitialSort;
Expand All @@ -132,6 +143,10 @@ public slots:
void OnDeleteServerAddrClicked();
void OnTimerPing();
void OnTimerReRequestServList();
void OnServerListItemSelectionChanged();
void OnAccessiblePreviousClicked();
void OnAccessibleNextClicked();
void OnToggleAccessibleClicked();

signals:
void ReqServerListQuery ( CHostAddress InetAddr );
Expand Down
Loading