Skip to content

Settings: choose which field is focused when the pills open - #116

Merged
krishhgg merged 1 commit into
krishhgg:mainfrom
hasin023:feat/default-focus-field
Oct 19, 2026
Merged

krishhgg merged 1 commit into
krishhgg:mainfrom
hasin023:feat/default-focus-field

Conversation

@hasin023

@hasin023 hasin023 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #115.

Adds a "Default field" setting (Days, Hours or Minutes) under Settings → Session. The pills open on that field for a start and for an extend. Hours stays the default, and a config from an earlier build keeps working (an unknown value falls back to Hours). I also added it to the settings list in docs/spec.md.

Tests: the config default, an older config, saving by name and a bad value, and the pills opening on each field for a start and an extend. I ran the config and UI tests locally and left the full suite to CI.

By hand on a MacBook Pro (M1), macOS 27.0.1, in a build with #93 on top: the picker shows in Settings and the app works. I did not try macOS 26.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

Adds a “Default field” picker under Settings → Session.

  • Duration entry opens on the field chosen in Session settings.

Tests and visual checks were not run. This Linux environment cannot run the native macOS app.

Reviews (3) · Last reviewed commit: "Settings: choose which field is focused ..." · Reviewed by Greptile

Adds a Default field setting (days, hours or minutes). The pills open on it
for a start and for an extend. Hours stays the default, and a config from an
earlier build keeps working.
@ne0cean

ne0cean commented Oct 14, 2026

Copy link
Copy Markdown

Pre-landing review: #116 "Settings: choose which field is focused when the pills open"

Tier: HIGH (7 files changed; one setting, read where the pills open; a persisted config key)
Invariant violations: none
Scope: CLEAN (the diff does what #115 asks: a Days/Hours/Minutes setting, Hours by default, nothing else)
Issues: 6 (CRITICAL 0, HIGH 0, MEDIUM 2, LOW 4)

What I checked

  • Every way the pills open. Clicking the idle eye, clicking again during a close, and Extend (customExtend) all go through StatusItemController.expand(mode:), which is the one line this PR changes (StatusItemController.swift:316). Start and Extend open on the same field. Typing still works from any starting field: typeDigit moves on to the next pill only when the current one is full, and it never moves past Minutes. Backspace moves back but never past Days. Tab, the arrow keys and d/h/m don't depend on where focus started.
  • Configs from earlier builds and bad values. This mattered more than it looks. When config.json fails to decode, SessionManager.swift:590 falls through to Config() and saves it, so one bad key would reset the user's whole config. Decoding this key with try? avoids that: an unknown or wrongly typed value becomes Hours, and the rest of the config is kept. A missing key also becomes Hours. Both cases have tests ({} and "seconds" next to endFloor: 15).
  • Saved form. The value is saved by name ("days", "hours", "minutes") through explicit init(from:)/encode(to:). These take precedence over the default Int raw-value coding, so reordering the cases won't change what's in config.json.
  • Tests. The UI test runs the real controller (Rig) through expand for every field in both modes. It would fail on the old hard-coded .hours for Days and Minutes, so it does catch the change. CI tested the merge with current main (dbcb42b = bfceef9 into b6a7499) on macOS 26 and on macOS 27. On both, all 4 new tests passed, along with 1239 tests (0 failures, 10 skipped, the usual tmux ones), and the release build with -warnings-as-errors passed.
  • Settings UI / accessibility. It's a standard SwiftUI Picker in the Session form. It uses the same generic bind(_:) as the other settings, which saves on change. VoiceOver reads it as "Default field, Hours, pop-up button", with the .help text as the hint. Nothing custom needs an accessibility label.

Findings

[MEDIUM] Process: no real-Mac report yet for bfceef9, and the hand check you mention doesn't meet CONTRIBUTING's rule
Why: CONTRIBUTING "Real-Mac evidence" asks for a clean checkout of the head commit "with no other pull requests applied", installed with ./scripts/install.sh, plus git rev-parse HEAD/git status --short/sw_vers/hw model. The PR body says the check was "in a build with #93 on top". merge-gate/evidence is still pending.
Fix: post a report for bfceef9 on a clean checkout. Steps: set each of Days/Hours/Minutes in Settings; click the eye, type a digit and check which pill takes it; do the same through Extend while a session runs. Include the log lines from that run.

[MEDIUM] docs/releasing.md: no "Real-Mac checks before a release" item for this behavior
Why: CONTRIBUTING says behavior that needs a real-Mac check adds an item to that list. Other Settings behavior is already listed there (for example the floor steppers, releasing.md:242). Which pill takes the first keystroke can't be seen from the unit tests alone.
Fix: add one line, e.g. "Default field: with Days, Hours and Minutes each chosen in Settings, the first digit typed after clicking the eye, and after Extend, lands in that pill; a config.json without the key opens on Hours."

[LOW] SettingsView.swift:136: tooltip uses "pill", which users don't see anywhere else
Why: "The pill that is focused when the pills open" is the only string in Sources/ that shows "pill" to the user. "Pill" is the design doc's word for these fields. Also, "Default field" sits right under "Default preset", where it reads like part of the preset. Apple's HIG asks for labels that say what the control does.
Fix (copy only, owner's call): e.g. label "Start typing in" or "Focus first", with help text "The duration field that is selected when you start or extend a session".

[LOW] Config.swift:151: an unknown value is dropped without a log line
Why: the app's other config.json fixes for hand edits are logged and written back (normalizeFloors -> config.json: ...; saved, SessionManager.swift:593-605). Here "seconds" quietly turns into Hours, and stays in the file until the user next changes a setting.
Fix (optional): log one line when the key is present but can't be decoded. It's fine as is if the maintainer prefers.

[LOW] Model now depends on a UI type
Why: before this PR nothing under Model/, Core/ or Store/ referred to DurationInput. Now Config stores DurationInput.Field, and its Codable conformance lives in UI/DurationInput.swift. It compiles, since it's one module and the file only imports Foundation, but it's a new dependency from Model to UI.
Fix (owner's call): leave as is, or move the conformance next to Config.

[LOW] ConfigTests: the saved names come from "\(field)", not written out as literals
Why: the test checks the JSON against Swift's description of the enum case, not against the names written out. If a case is renamed, the test fails even though the saved name hasn't changed. And the test never states the saved format directly.
Fix: compare against ["days", "hours", "minutes"] written out.

Conventions

The commit message follows the repo's "Area: summary" style. The change has tests as CONTRIBUTING asks, and spec.md is updated in both places (settings list and pill behavior). Not done yet: the real-Mac report (above), and the owner's accepted on #115, which has had no owner reply so far. The merge-gate statuses for accepted, evidence and codex are still pending at bfceef9.

Not verified here

I couldn't run swift build/swift test on this machine: its Command Line Tools fail to compile the manifest (.macOS(.v26)/.swiftLanguageMode don't resolve against its PackageDescription), which is a local toolchain problem and not this PR's. The test results above come from the CI logs for this head, not from a local run.

Status: DONE_WITH_CONCERNS (the code is ready; the open items are process and copy)

Verdict: APPROVE on the code. Merging is up to the maintainer and the merge gate.
Reviewed-Head: bfceef9

SergeiNikolenko added a commit to SergeiNikolenko/Insomnia that referenced this pull request Oct 16, 2026
@krishhgg

Copy link
Copy Markdown
Owner

End-to-end test in a VM on macOS 27.0.1

The new Default field setting works. With the default (Hours), typing 5 and Enter starts a 5 hour session, as on main. After choosing Days in Settings, the pills open with Days focused, and the same 5 and Enter starts a 5 day session. The choice is saved to config.json as "defaultFocusField": "days".

Setup

A Tart VM on the maintainer's Mac mini, macOS 27.0.1 (26A434) with Xcode 27.0. Each version was built from source and installed with scripts/install.sh, then driven by a script inside the VM using Accessibility and posted mouse events.

Main (76e517d) against this PR's head (bfceef9). The script (t116.sh) clicked the eye, typed 5, pressed Enter and read the session length from session.json. On the PR it then opened Settings, chose a different value in the Default field pop-up (Days), and did it again.

Results

Focused pill Typed 5, Enter Session length
main Hours 5 in Hours 18000 s (5 h)
this PR, Default field: Hours (default) Hours 5 in Hours 18000 s (5 h)
this PR, Default field: Days Days 5 in Days 432000 s (5 days)

The pills as they open, main, then this PR with Hours, then this PR with Days:

main pills

PR hours pills

PR days pills

After typing 5, with Hours and then with Days:

PR hours typed

PR days typed

The setting in Settings:

settings

Not covered

  • Minutes. The script chose Days; Minutes was not tried.
  • macOS 26 and real hardware.

The full log is run-full-lab27.log. In it, the second PR run is labeled minutes, but the pop-up value chosen was Days, as its "Default field after" line says.

🤖 Generated with Claude Code

@krishhgg krishhgg closed this Oct 18, 2026
@krishhgg krishhgg reopened this Oct 18, 2026
@krishhgg
krishhgg marked this pull request as draft October 18, 2026 02:32
@krishhgg
krishhgg marked this pull request as ready for review October 18, 2026 02:32
@krishhgg krishhgg added the accepted The owner accepted this pull request; merge-gate/accepted reads it. Only the owner adds it. label Oct 19, 2026
@krishhgg krishhgg closed this Oct 19, 2026
@krishhgg krishhgg reopened this Oct 19, 2026
@krishhgg
krishhgg marked this pull request as draft October 19, 2026 05:57
@krishhgg
krishhgg marked this pull request as ready for review October 19, 2026 05:57
@krishhgg
krishhgg merged commit 0f71cbf into krishhgg:main Oct 19, 2026
37 of 44 checks passed
nsd97 added a commit to nsd97/Insomnia that referenced this pull request Oct 19, 2026
Conflicts:
- SettingsView.swift: main added a Default field picker (krishhgg#116), the
  version line and the update check (krishhgg#117) to the old layout. They sit
  in General on the one page: Default field after Default session
  length, the version and the update check after Open at login, as
  main ordered them. releaseCheck is main's, unchanged.
- InsomniaApp.swift: SettingsView without AnyView, as this branch has
  it, with main's ReleaseChecker passed in.
- docs/spec.md: both edits to the presets line.

Also from the merge: the Settings window test passes a ReleaseChecker,
and main's "Settings > App" in README.md, docs and ReleaseCheck.swift
reads "Settings > General", the section's name on the one page.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted The owner accepted this pull request; merge-gate/accepted reads it. Only the owner adds it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a setting for which field is focused when the pills open

3 participants