Skip to content

feat(a11y): QAccessibleInterface for VfoWidget + LevelBar (#3754)#3758

Merged
ten9876 merged 2 commits into
mainfrom
a11y/vfowidget-levelbar-3754
Jun 23, 2026
Merged

feat(a11y): QAccessibleInterface for VfoWidget + LevelBar (#3754)#3758
ten9876 merged 2 commits into
mainfrom
a11y/vfowidget-levelbar-3754

Conversation

@ten9876

@ten9876 ten9876 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Closes #3754.

Implements QAccessibleInterface for the two custom-painted widgets the Qt Accessibility Static Analysis flagged (both pre-existing; #3751 surfaced them by touching the file).

VfoWidget → VfoWidgetAccessible (role Grouping)

The VFO flag custom-paints its chrome and, while collapsed, the slice-letter and TX badges — which have no accessible child-widget equivalent, so a screen reader on a collapsed flag got nothing. Added a text(QAccessible::Name) summary built from a new VfoWidget::accessibleSummary():

"VFO slice A, 14.225000 MHz, transmit slice, collapsed"

Expanded-mode detail (frequency, dBm) is already announced via the child labels' QAccessibleValueChangeEvents, so this fills the collapsed-mode gap and gives the group a meaningful name for navigation.

LevelBar → LevelBarAccessible (NoRole)

LevelBar is the render-only ESC combiner level meter. Following the existing PhaseKnob precedent (whose comment states "the named ESC phase/gain sliders are the accessible interface"), the bar is decorative — the ESC gain/phase sliders carry the accessible semantics — so it returns QAccessible::NoRole to drop out of AT navigation. Matched by objectName since LevelBar is a .cpp-local class with no Q_OBJECT of its own.

Notes

  • Factory registered once via QAccessible::installFactory, mirroring PhaseKnob.cpp.
  • API usage validated with a standalone -fsyntax-only compile against Qt6; follows the established a11y pattern in docs/a11y.md.
  • Live re-announcement on TX-toggle/collapse (firing QAccessible::NameChanged) is a possible future enhancement — out of scope here; the summary is correct on query/navigation.

🤖 Generated with Claude Code

The Qt Accessibility Static Analysis flagged two custom-painted widgets in
VfoWidget that lacked a QAccessibleInterface, so screen readers couldn't
read their content. Both are pre-existing; #3751 surfaced them by touching
the file.

- VfoWidget: custom-paints its chrome and, while collapsed, the slice-letter
  and TX badges (which have no child-widget equivalent). Add VfoWidgetAccessible
  (role Grouping) with a live text(Name) summary — slice letter, frequency, TX
  state, collapsed — so the flag isn't opaque to AT tools. Expanded-mode detail
  (frequency, dBm) is already announced via the child labels' value-change events.

- LevelBar: the render-only ESC combiner level meter. Like PhaseKnob, the named
  ESC gain/phase sliders are the accessible interface, so the bar is decorative
  and returns NoRole to drop out of AT navigation. Matched by objectName since
  LevelBar has no Q_OBJECT of its own.

Factory registered once via QAccessible::installFactory, mirroring PhaseKnob.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ten9876 ten9876 requested a review from a team as a code owner June 23, 2026 02:58
@github-actions

Copy link
Copy Markdown

Accessibility check findings

This PR touches src/gui/ files with 1 accessibility issue(s).
Full details appear as inline annotations on the diff above.

Files scanned : 2
Files with findings: 1
Total findings : 1

See docs/a11y.md for the canonical
Qt patterns (setAccessibleName, QAccessibleValueChangeEvent,
QAccessibleInterface subclasses, etc.). Structural remediation
(keyboard navigation, custom QAccessibleInterface subclasses)
is tracked in #3288.

Findings are warnings only — the build is not blocked.

…3754)

The a11y linter (tools/check_a11y.py) scans each changed file independently
and clears a custom-painted widget only when the file itself names
QAccessibleInterface or has a companion *Accessible.h. VfoWidget.cpp already
satisfies this (the factory returns QAccessibleInterface*), but VfoWidget.h
did not, so the header — touched by this PR — stayed flagged.

Expand the accessibleSummary() doc comment to point at VfoWidgetAccessible
(the QAccessibleInterface in VfoWidget.cpp). Accurate documentation for a
header reader, and it clears the lint. Verified: check_a11y.py now reports
0 findings on both files.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @ten9876 — clean, well-scoped a11y work that closely mirrors the existing PhaseKnob precedent. I checked the key correctness points and they hold up:

  • Factory key is correct. VfoWidget has Q_OBJECT and lives in namespace AetherSDR, so metaObject()->className() is "AetherSDR::VfoWidget" — the vfoAccessibleFactory key match will fire. (Matches how phaseKnobFactory keys on "AetherSDR::PhaseKnob".)
  • Units are right. SliceModel::frequency() returns MHz (SliceModel.h:38), so the ", %1 MHz" summary is accurate — no Hz→MHz mismatch.
  • Null safety on m_slice in accessibleSummary() is handled.
  • LevelBar matched by objectName is the right call given it has no Q_OBJECT; setObjectName(...) runs in buildTabContent(), well before any AT query, so the lazy lookup resolves fine.
  • Single-install static guard mirrors PhaseKnob exactly.

Two minor, optional notes (neither blocking):

  1. Dead branch in accessibleSummary(). SliceModel::letter() never returns empty — it falls back to 'A' + m_id (SliceModel.h:34-36). So the letter.isEmpty() ? "VFO" : "VFO slice %1" ternary's first arm is unreachable. Harmless, but you could drop it for clarity (the !m_slice guard above already covers the no-slice case with "VFO flag").

  2. Translatable text. The summary uses QStringLiteral rather than tr(). Since this is spoken AT text, tr() would be marginally more correct for future localization — though the codebase is inconsistent here (the VfoWidget itself mixes tr("Signal meter") with plain-literal accessible names), so this matches local convention either way.

LGTM. Nice fill of the collapsed-flag gap.


🤖 aethersdr-agent · cost: $3.8736 · model: claude-opus-4-8

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the diff against the codebase. This is a clean, well-scoped accessibility addition — thanks @ten9876.

What I verified

  • Follows the established precedent. LevelBarAccessible / the factory registration mirror PhaseKnob.cpp (static-flag-guarded installFactory, NoRole to drop a decorative render-only widget from AT navigation) almost line-for-line. Good consistency.
  • Frequency unit is correct. SliceModel::frequency() returns MHz (SliceModel.h:38), so arg(..., 0, 'f', 6) yields 14.225000 as the description shows — not a Hz value. ✅
  • No null-deref / leak risk. accessibleSummary() guards m_slice; the VfoWidget factory branch only runs on a key match (so the qobject_cast<QWidget*> is always non-null); Qt owns the returned interface. Summary is computed at query time, so it's always fresh.
  • No Qt-floor concern. Only long-standing APIs (QAccessibleWidget, installFactory, NoRole, the Grouping ctor) — nothing Qt 6.5+, consistent with the Linux CI floor.
  • LevelBar objectName tagging is safe. LevelBar is fully custom-painted (no QSS selectors keyed on objectName), so setObjectName() has no styling side effects.

Minor, non-blocking

  • Matching LevelBar by objectName is the right call given it has no Q_OBJECT of its own, but it's a slightly looser contract than the class-name key path — if anything else ever reuses "AetherSDR.LevelBar" the factory would wrap it. The namespaced constant makes a collision unlikely, so this is fine as-is; just worth a mental note.
  • The out-of-scope deferral of live NameChanged re-announcement on TX-toggle/collapse is the right scoping, and you flagged it honestly. A collapsed flag's summary being correct on query/navigation already closes the reported gap.

Looks good to merge from my side. Nice, focused fix.


🤖 aethersdr-agent · cost: $5.1176 · model: claude-opus-4-8

@ten9876 ten9876 merged commit 4dad9a2 into main Jun 23, 2026
6 checks passed
@ten9876 ten9876 deleted the a11y/vfowidget-levelbar-3754 branch June 23, 2026 11:53
ten9876 added a commit that referenced this pull request Jun 23, 2026
…rites (rebased #3765) (#3771)

Closes #3762. **Rebased + conflict-resolved version of #3765** by
@dsocha (original authorship preserved on all three commits). Opened
from a maintainer branch because the original is on a fork; #3765 should
be closed in favor of this.

## What it fixes (unchanged from #3765)
Two GPU-flag-sprite rendering bugs in the SmartMTR meter view (#3723):
1. **Extremes value labels never drew (#3762)** — `drawSmartMtrLabels()`
gated on `m_smartMtrWidget->isVisible()`, false in GPU-sprite mode. Now
gates on `!size().isEmpty()` (the same condition the sprite is drawn
under), so labels track the markers in both GPU and software paths.
2. **Meter selector vanished from the sprite** — `QWidget::render()`
can't rasterize a `QGraphicsEffect`. The selector now gates its
underline on an explicit `m_meterMenuOpen` flag (not the hidden row's
`isVisible()`), and the option rows drop the `QGraphicsOpacityEffect`s
in favor of `setEnabled(false)` + `:disabled` stylesheet dimming (new
`QComboBox:disabled` rule in `ComboStyle.h`), so they stay visible
**and** dimmed in the sprite.

## Rebase / conflict resolution (the reason this is a new branch)
The original #3765 conflicted with **#3760** ("disable identity opacity
effect…"), which merged into `main` after #3765 was branched. Both
target the same blank-rows-in-sprite bug, but:
- **#3760** only neutralized the effect at *identity* (opacity 1.0) —
i.e. for *enabled* rows. A *disabled* row still carried an active 0.45
effect, so it would **still render blank** in a sprite.
- **#3765** removes the effects entirely, so both enabled and disabled
rows render. **#3765#3760.**

Resolved `syncSmartMtrSettingsState` in favor of #3765's approach and
**dropped #3760's now-dead `setRowOpacity` lambda + the `m_*Fade`
members**. Verified: no dangling `m_*Fade`/`kDisabledOpacity`
references; the `QGraphicsOpacityEffect` on the DSP filter-level row
(meant to vanish in sprites) is untouched.

## Verification
- Rebased onto `main` (`437f7469`); conflict resolved; commits signed.
- Composes with the merged #3752 (M3 `syncSmartMtrSettingsControls`
re-seeds combos, then `syncSmartMtrSettingsState` disables the rows) and
#3758 (a11y — `QAccessibleInterface` reference preserved; a11y linter
passes 0 findings).
- Original hardware test (macOS / FlexRadio 6000) per #3765.

Credit: @dsocha.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Dariusz Socha <dariusz.socha.pl@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

a11y: VfoWidget + LevelBar custom-painted widgets need QAccessibleInterface

1 participant