Redesign TX strip (3a) + Band & Mode sheet (3b) - #762
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Hunt selection has incorrect edge-case behavior, insufficient engine-level coverage, and several tap targets below the stated minimum.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Redesigns the Compose TX controls and adds configurable Hunt candidate ranking.
Changes:
- Reworks the TX strip, status timer, CAT chip, and band/mode sheet.
- Adds persistent Hunt priorities and smart filters.
- Adds unit tests for extracted UI and ranking logic.
File summaries
| File | Description |
|---|---|
TxStatusVisualsTest.kt |
Tests TX status mapping. |
SlotTimerStateTest.kt |
Tests slot and next-window timing. |
CatStatusChipLogicTest.kt |
Tests CAT labels and visuals. |
BandModeSheetLogicTest.kt |
Tests band/mode helpers. |
HuntSelectionTest.kt |
Tests Hunt ranking and filtering. |
strings_compose.xml |
Adds redesigned UI strings. |
TxStrip.kt |
Implements the redesigned TX controls. |
SlotTimerBar.kt |
Adds progress and next-window components. |
CatStatusChip.kt |
Adds plain-language connection labels. |
BandModeSheet.kt |
Adds band/mode and Hunt sheets. |
HuntSelection.kt |
Defines Hunt priorities and ranking. |
FT8AFApp.kt |
Integrates sheets and persisted settings. |
GeneralVariables.java |
Stores shared Hunt configuration. |
FT8TransmitSignal.java |
Applies candidate filters and ranking. |
DatabaseOpr.java |
Loads persisted Hunt settings. |
Review details
Suppressed comments (2)
ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/components/TxStrip.kt:653
- Each TX-period segment is only 36dp high, below the PR's 44dp minimum tap target. The containing card has room for 44dp segments, so expand the clickable boxes.
.height(36.dp)
ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/components/BandModeSheet.kt:575
- The minimum-signal chips are only 40dp high, below the stated 44dp minimum tap target. Increase the clickable height to at least 44dp.
.height(40.dp)
- Files reviewed: 15/15 changed files
- Comments generated: 8
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Hunt priority and smart filters (min signal, avoid pileups) now apply only while Hunt (autoFollowCQ) is on; standalone auto-call-follow keeps Latest with no filtering so a followed station is never suppressed by a Hunt-only setting. - HuntCandidate.snr is nullable: an unknown SNR can win neither Strongest nor Weakest (it used to map to MIN_VALUE and always win Weakest); both fall back to Latest when no candidate has an SNR. - huntDistanceKm parses both grids explicitly and returns null when either fails — MaidenheadGrid.getDist(String,String) returns 0 for a bad grid, which made malformed grids rank as 0 km. - Engine glue extracted into package-private statics (effectiveHunt*, collectAnsweredCallsigns, buildHuntCandidates, huntDistanceKm) and covered by FT8TransmitSignalHuntTest (Robolectric). - Band sheet daytime hint re-evaluated each time the sheet opens (remember keyed on visibility) via a tested isDaytimeHour(). - Hunt priority / min-signal chips raised to 44dp; HuntChip comment documents the platform 48dp touch-target expansion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reworks the TX strip to the high-fidelity design (option 3a) and adds the
Band & Mode bottom sheet (option 3b). Wires into dev's existing Hunt-options
feature rather than duplicating it.
TX strip (TxStrip.kt):
- Status row: pulse dot + Listening/Transmitting/Tuning + live, parity-aware
"Next transmit window in Xs" countdown, and the CAT chip.
- 48dp BAND & MODE row ("14.074 MHz · 20m · FT8") opening the new sheet.
- 72dp Call CQ with a visible MORE split-button (replaces long-press-only;
opens the existing CQ-variants menu).
- Hunt tile whose amber chip shows the active hunt priority (or "Off") and
opens dev's HuntOptionsSheet; tapping the tile toggles Hunt.
- TX PERIOD segmented control (1st even / 2nd odd) replacing the TX1 toggle.
- Tune / DX buttons; inline volume slider preserved.
Band & Mode sheet (BandModeSheet.kt): FT8/FT4/FT2 mode toggle with cycle
times, curated most-used band list from assets/bands.txt with plain-English
hints (20m is time-of-day aware), and an "All bands & custom frequency"
footer → the existing full picker.
CatStatusChip now reads "Radio linked" / "Radio not linked" (Inter).
SlotTimerBar gains NextTxWindowLabel + the pure, tested nextTxWindowSeconds;
dev's slot bar + clock-sync pill are left in place above the strip.
Unit tests for the extracted pure logic (nextTxWindowSeconds, txStatusVisuals,
formatSlotSeconds, bandHintRes, mostUsedBands, cat chip labels). Verified on
emulator: strip, both sheets, TX-period toggle, MORE menu, Hunt tile opening
dev's sheet. All tap targets >= 44dp; px→dp 1:1.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7605c5f to
7834115
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #762 +/- ##
============================================
- Coverage 41.58% 41.41% -0.18%
Complexity 290 290
============================================
Files 282 283 +1
Lines 33350 33624 +274
Branches 4017 4055 +38
============================================
+ Hits 13870 13924 +54
- Misses 19124 19343 +219
- Partials 356 357 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Dial selection can be displayed incorrectly, and several redesigned controls remain inaccessible or unusable on constrained displays.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/components/BandModeSheet.kt:311
- The checkmark and highlight are visual only, so TalkBack cannot tell which dial is currently selected. Give each row selectable/radio-button semantics with
selected = selectedrather than a plainclickable.
.clickable { onClick() }
ft8af/app/src/main/kotlin/radio/ks3ckc/ft8af/ui/components/TxStrip.kt:694
activeonly changes colors, so TalkBack cannot announce whether the Tune/DX toggle is on or off (especially DX, whose label never changes). Use toggleable semantics or expose a localized state description tied toactive.
.clickable(enabled = enabled) { onClick() }
- Files reviewed: 10/10 changed files
- Comments generated: 6
- Review effort level: Balanced
…wrap
- BandModeSheet: mark the selected band by exact bandIndex, not wavelength, so
an alternate dial (e.g. 20m 14.090) no longer lights up the curated 14.074 row.
- BandModeSheet: mode segments and band rows use selectable(role = RadioButton)
so TalkBack announces the active mode/band, not just a click target.
- TxStrip: TX-period segments use selectable(role = RadioButton); Tune/DX use
toggleable so the on/off state is announced (DX's label never changes).
- TxStrip: the Tune countdown ("Tune 60s") wraps to a second line instead of
clipping on a narrow phone.
- TxStrip: the Hunt chip exposes its visible label ("Off"/priority) as a
stateDescription so a screen reader hears the current option.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reworks the TX strip to the high-fidelity design (option 3a) and adds the Band & Mode bottom sheet (option 3b) from
design_handoff_tx_strip_redesign. Rebased onto currentdevand wired into dev's existing Hunt-options feature (this PR does not duplicate it).TX strip (
TxStrip.kt)HuntOptionsSheet; tapping the tile toggles Hunt.Band & Mode sheet (
BandModeSheet.kt)assets/bands.txtwith plain-English hints (20m is time-of-day aware) and an "All bands & custom frequency" footer → the existing full picker.Other
CatStatusChipnow reads "Radio linked" / "Radio not linked" (Inter).SlotTimerBargainsNextTxWindowLabel+ the pure, testednextTxWindowSeconds; dev's slot bar + clock-sync pill are left in place above the strip.FT8AFIconsreused; all tap targets ≥44dp; px→dp 1:1.Scope note
An earlier revision of this branch was cut from a stale
mainand also re-implemented the Hunt priority/smart-filters feature — which already exists ondev. That duplication has been dropped; this PR is now redesign-only and layers on dev's hunt backend.Tests
Unit tests for the extracted pure logic:
nextTxWindowSeconds,txStatusVisuals,formatSlotSeconds,bandHintRes,mostUsedBands, and the CAT chip labels. FulltestDebugUnitTestgreen.Verification
Built and driven on an emulator against
dev: redesigned strip, Band & Mode sheet, TX-period toggle, MORE menu, and the Hunt tile opening dev's Hunt sheet — with dev's clock-sync pill and slot bar intact.🤖 Generated with Claude Code