Skip to content

focus: say whose tab this is, and fold the engineer's half away - #40

Merged
widgetii merged 2 commits into
mainfrom
focus-ux
Sep 25, 2026
Merged

widgetii merged 2 commits into
mainfrom
focus-ux

Conversation

@widgetii

Copy link
Copy Markdown
Member

Read off screenshots of the Focus tab on an 85H50AI at 1440×900, 1366×768 and 390×844, from an owner's seat rather than from the source. The tab put two jobs on one panel at one weight — an owner's, make the picture sharp, and an ISP engineer's, tune what the camera counts as sharp — and the engineer's won the first impression.

What the screenshots showed

  • On a stock camera (isp.af unset, the profile's IIR1 in force) the panel opened with a yellow box: "3 zones are reading at the top of the camera's counter … Lower the first gain until this clears." An owner reads "my camera is broken", and nothing on screen was called the first gain.
  • Three numbers were all called sharpest: the outlined 3×3 block said 14 793, the sentence above it said 56 099 at "row 5, column 4" — a cell of the 15×17 grid that view does not draw — and the 4×4 said 24 553 of the same star chart.
  • Near and Far were two bare words, 32px tall on a desktop and the smallest thing on the panel; nothing said hold, and a refused move let go in silence.
  • Fourteen bare number boxes and four verbs sat straight under them, and on the 1366×768 laptop the fold landed exactly on GAINS.
  • Measure by hand told the owner of a sealed zoom block to turn a barrel.
  • On the phone a tapped tab scrolled itself into view and took Back off the left edge (x = −259) with no scrollbar to say so.
  • The tab could be opened before the first frame landed, and showed the designer's form with every box empty.

What changes

  • The panel opens by sending an owner to the live page, when the host names one (new optional focus.liveHref), and says what this tab is for.
  • The filter designer is a closed <details>, with every box named (Scale, 1a…3b; Threshold / Slope / Limit) and an aria-label on each.
  • The counter warning moves beside the gain it names; the grid status states the count as a fact in the same sentence as "too dark" and "blown out".
  • The status line reports the outlined block in the readout's own words and value ("Sharpest at top left: 14 793"), with a block "best so far" that only a reading advances. With every zone drawn it reports the peak zone, as before.
  • Near and Far wear the live page's icons and names, are 44px at every width, say what they are doing while held, and say why the lens did not move when the host refuses.
  • "Try it" carries the trial's length; a line under the verbs says Measure it moves the lens out and back.
  • The hand sweep's prompt speaks in the motor's terms where there is one.
  • On the phone, Back sticks to the left edge and the bar fades where there is more of it.
  • A frame not yet captured is said, rather than a grid drawn around nothing.

Testing

tests/ui-check.html: seventeen new checks, watched red on the previous build first (22 reds, the rest being "best seen" renamed to "best so far"). Full loop clean: node --check, ./tools/build.sh, dist/ in sync, tools/smoke.mjs, tools/ui-check.mjs at both widths.

Rendered on the lab 85H50AI with this build served locally in place of the CDN, at the three sizes above: no script errors, the panel reads as intended, and on the phone the bar reports "more to the right" and keeps Back on screen.

Read off screenshots of an 85H50AI at 1440x900, 1366x768 and 390x844
rather than off the source: the Focus tab put two jobs on one panel at one
weight -- an owner's, make the picture sharp, and an ISP engineer's, tune
what the camera counts as sharp -- and the engineer's won the first
impression.

- On a stock camera (isp.af unset, the profile's IIR1 in force) the panel
  opened with a yellow box: "3 zones are reading at the top of the camera's
  counter ... Lower the first gain until this clears." An owner reads "my
  camera is broken", and nothing on screen was called the first gain.
- Three numbers were all called sharpest: the outlined 3x3 block said
  14 793, the sentence above it said 56 099 at "row 5, column 4" -- a cell
  of the 15x17 grid that view does not draw -- and the 4x4 said 24 553 of
  the same star chart.
- Near and Far were two bare words, 32px tall on a desktop and the smallest
  thing on the panel; nothing said hold, and a refused move let go in
  silence.
- Fourteen bare number boxes and four verbs sat straight under them, and on
  the 1366x768 laptop the fold landed exactly on GAINS.
- Measure by hand told the owner of a sealed zoom block to turn a barrel.
- On the phone a tapped tab scrolled itself into view and took Back off the
  left edge (x=-259) with no scrollbar to say so.
- The tab could be opened before the first frame landed, and showed the
  designer's form with every box empty.

So: the panel opens by sending an owner to the live page, when the host
names one (`focus.liveHref`), and says what this tab is for. The designer is
a closed <details> with every box named. The counter warning moves beside
the gain it names (Scale) and the grid status states the count as a fact.
The status line reports the outlined block in the readout's own words and
value, with a block "best so far" that only a reading advances. Near and
Far wear the live page's icons and names, are finger-sized at every width,
say what they are doing and why the lens did not move. Try it carries the
trial's length and Measure it its cost. The hand sweep speaks in the
motor's terms where there is one. Back sticks on the phone and the bar
fades where there is more of it. A missing frame is said rather than drawn
around.

tests/ui-check.html: seventeen new checks, watched red on the previous
build first (22 reds, the rest being "best seen" renamed to "best so far").
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Clarify Focus workflow and hide advanced filter tuning

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Direct camera owners to live focusing and explain the diagnostic tab’s purpose.
• Align focus readouts, lens feedback, and warnings with visible controls.
• Fold advanced tuning by default and preserve mobile navigation accessibility.
Diagram

graph TD
  Owner["Camera owner"] --> Focus["Focus tab"] --> Frame{"Frame ready?"}
  Focus -->|"liveHref"| Live["Live page"]
  Frame -->|"Yes"| Readout["Zone readout"]
  Frame -->|"No"| Waiting["Waiting note"]
  Focus --> Lens["Lens controls"]
  Focus --> Filter["Advanced filter"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Separate advanced tuning tab
  • ➕ Completely isolates owner and ISP-engineer workflows
  • ➕ Provides more space for filter explanations and controls
  • ➖ Adds navigation complexity and another top-bar overflow contributor
  • ➖ Makes related readout and tuning controls harder to compare
2. Automatically redirect to the live page
  • ➕ Moves owners immediately to the primary focusing workflow
  • ➕ Further reduces ambiguity about the tab’s purpose
  • ➖ Unexpected navigation could discard editor context
  • ➖ Prevents intentional access to diagnostic focus readings

Recommendation: Keep the PR’s progressive-disclosure approach: an explicit live-page link preserves user control, while the native details foldout separates advanced tuning without fragmenting related diagnostics. A separate tab or automatic redirect would create more navigation cost than clarity.

Files changed (5) +811 / -163

Enhancement (3) +635 / -160
editor.cssShip responsive Focus control and navigation styles +21/-0

Ship responsive Focus control and navigation styles

• Updates the CDN-ready stylesheet with 44px lens controls, accessible link colors, a sticky mobile Back button, and an overflow fade for hidden tab content. Mirrors the source stylesheet used by the build.

dist/editor.css

editor.jsShip the redesigned Focus workflow +307/-80

Ship the redesigned Focus workflow

• Publishes the built editor logic for live-page guidance, frame-aware messaging, readout-aligned best values, descriptive lens feedback, and collapsed filter tuning. Includes named filter controls and contextual saturation guidance.

dist/editor.js

editor.jsSeparate owner focusing from advanced ISP tuning +307/-80

Separate owner focusing from advanced ISP tuning

• Adds optional live-page guidance, no-frame messaging, readout-consistent sharpness values, per-granularity best tracking, and visible lens movement errors. Moves labeled filter controls into a closed advanced disclosure and relocates saturation advice beside Scale.

src/editor.js

Bug fix (1) +21 / -0
editor.cssImprove Focus controls and mobile tab navigation +21/-0

Improve Focus controls and mobile tab navigation

• Makes Near and Far controls finger-sized while preserving their labels at narrow widths. Keeps Back visible in horizontally scrolling tab bars, signals additional content, and improves panel-link contrast.

src/editor.css

Tests (1) +155 / -3
ui-check.htmlCover Focus semantics, accessibility, and responsive behavior +155/-3

Cover Focus semantics, accessibility, and responsive behavior

• Adds browser checks for sticky mobile navigation, live-page guidance, waiting-frame messaging, block-aligned readouts, labeled advanced inputs, lens feedback, warning placement, and motor-aware sweep instructions. Renames existing assertions from “best seen” to “best so far.”

tests/ui-check.html

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Old lens failures reach a new panel ✓ Resolved 🐞 Bug ☼ Reliability
Description
The keyboard handler now passes moveRefused to moveSend, but leaving Focus does not advance
moveGen when no pointer hold is active. If that asynchronous nudge rejects after the user leaves
and reopens Focus, its callback writes the previous request’s failure into the newly built movement
status.
Code

src/editor.js[4363]

+			moveSend(verb, moveRefused);
Evidence
moveSend invokes a failure callback only when its captured generation remains current, but
moveRelease returns without incrementing that generation when no hold exists. Mode teardown calls
that ineffective path and clears moveSay, while reopening Focus installs a new moveSay that the
still-current keyboard rejection can update.

src/editor.js[4220-4233]
src/editor.js[4242-4248]
src/editor.js[4263-4272]
src/editor.js[4360-4363]
src/editor.js[5069-5071]
src/editor.js[5146-5150]
src/editor.js[5433-5439]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A delayed keyboard movement rejection remains current across Focus teardown and can write its error into a subsequently rebuilt panel.
## Fix Focus Areas
- src/editor.js[4220-4233]
- src/editor.js[4360-4363]
- src/editor.js[5433-5439]
- dist/editor.js[4220-4233]
- dist/editor.js[4360-4363]
- dist/editor.js[5433-5439]
## Recommended Fix
Invalidate the movement generation unconditionally when leaving Focus, rather than relying on `moveRelease` to do so only for an active pointer hold. Ensure delayed keyboard callbacks cannot match the generation of a newly built panel, then regenerate the distribution build.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Returning to a block view revives an old record 🐞 Bug ≡ Correctness
Description
noteBlockBest() returns before changing blockBestGrain when the operator selects All zones,
leaving the prior 3×3 or 4×4 record cached. Switching from a block view to All zones and back before
the next block reading makes the retained grain match focusBlocks again, so rendering displays the
pre-switch value as “best so far” even though the new readout should start fresh.
Code

src/editor.js[R4098-4103]

+	function noteBlockBest(sum) {
+		if (!focusBlocks) return;
+		const c = coarsen(sum, focusBlocks, { detail: focusBest ? zoneDetail(focusBest) : null });
+		const v = c.best === null ? null : c.blocks[c.best].value;
+		if (blockBestGrain !== focusBlocks) { blockBest = null; blockBestGrain = focusBlocks; }
+		if (v !== null && (blockBest === null || v > blockBest)) blockBest = v;
Evidence
The All-zones branch (focusBlocks === 0) exits before either block-history field is updated, while
the grain selection handler only changes focusBlocks and rerenders without clearing the cache.
Rendering exposes the cached record whenever blockBestGrain === focusBlocks, so reselecting the
previous block size displays that retained value before any new block reading arrives.

src/editor.js[4098-4103]
src/editor.js[5163-5177]
src/editor.js[4138-4143]
src/editor.js[4133-4143]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Selecting **All zones** does not invalidate the stored block-readout record because `noteBlockBest()` returns before updating its grain marker. Returning to 3×3 or 4×4 can therefore show a value measured before the readout changed as the new view's “best so far.”
## Fix Focus Areas
- src/editor.js[4098-4103]
- src/editor.js[5163-5177]
- dist/editor.js[4098-4103]
- dist/editor.js[5163-5177]
## Recommended Fix
When the selected readout changes, immediately clear `blockBest` and invalidate `blockBestGrain`, including when selecting All zones, so only a subsequent poll can establish a fresh record for a block-based readout. Alternatively, make the All-zones path assign `blockBestGrain` a distinct value that cannot match a block readout; apply the source change and regenerate the distribution build.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Narrow screens initially hide extra controls ✓ Resolved 🐞 Bug ≡ Correctness
Description
moreBar() is registered only as a scroll listener and is not called after the top bar is
populated. When the bar first overflows and no resize or scroll has occurred, .re-more is absent,
so the hidden-scrollbar layout does not show the new right-edge fade that indicates additional
controls.
Code

src/editor.js[R414-417]

+	const moreBar = () => {
+		top.classList.toggle('re-more', top.scrollLeft + top.clientWidth < top.scrollWidth - 1);
+	};
+	top.addEventListener('scroll', moreBar, { passive: true });
Evidence
The added code defines the class toggle and attaches it only to scrolling. The top bar's children
are appended later, while the CSS applies the fade exclusively when that class exists; therefore
initial overflow has no affordance until another event happens to invoke the helper.

src/editor.js[414-417]
src/editor.js[456-459]
src/editor.js[837-845]
src/editor.css[257-269]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The overflow affordance is calculated only after a scroll or resize event. A newly mounted narrow editor can therefore overflow without receiving the `.re-more` class and without displaying the right-edge fade.
## Fix Focus Areas
- src/editor.js[414-417]
- src/editor.js[456-459]
## Recommended Fix
Call `moreBar()` once after all top-bar children have been appended, ideally after layout is available (for example, via a queued animation frame). Keep the existing scroll and resize listeners to recalculate it after later layout changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/editor.js
Comment thread src/editor.js
Comment on lines +4098 to +4103
function noteBlockBest(sum) {
if (!focusBlocks) return;
const c = coarsen(sum, focusBlocks, { detail: focusBest ? zoneDetail(focusBest) : null });
const v = c.best === null ? null : c.blocks[c.best].value;
if (blockBestGrain !== focusBlocks) { blockBest = null; blockBestGrain = focusBlocks; }
if (v !== null && (blockBest === null || v > blockBest)) blockBest = v;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Returning to a block view revives an old record 🐞 Bug ≡ Correctness

noteBlockBest() returns before changing blockBestGrain when the operator selects All zones,
leaving the prior 3×3 or 4×4 record cached. Switching from a block view to All zones and back before
the next block reading makes the retained grain match focusBlocks again, so rendering displays the
pre-switch value as “best so far” even though the new readout should start fresh.
Agent Prompt
## Issue description
Selecting **All zones** does not invalidate the stored block-readout record because `noteBlockBest()` returns before updating its grain marker. Returning to 3×3 or 4×4 can therefore show a value measured before the readout changed as the new view's “best so far.”

## Fix Focus Areas
- src/editor.js[4098-4103]
- src/editor.js[5163-5177]
- dist/editor.js[4098-4103]
- dist/editor.js[5163-5177]

## Recommended Fix
When the selected readout changes, immediately clear `blockBest` and invalidate `blockBestGrain`, including when selecting All zones, so only a subsequent poll can establish a fresh record for a block-based readout. Alternatively, make the All-zones path assign `blockBestGrain` a distinct value that cannot match a block readout; apply the source change and regenerate the distribution build.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/editor.js
… zones, and the bar's fade

Three findings from the automated review of #40: two taken as written, one
that did not reproduce.

1. A keyboard nudge holds nothing, so leaving Focus with one unanswered left
   its generation current, and a refusal arriving after the panel had been
   rebuilt wrote into the new panel's line. setMode now bumps the generation
   on the way out, as a release does.

2. The block "best so far" stopped advancing under All zones, so coming back
   to 3x3 showed the record as it stood at the switch and called a peak swept
   past under All zones never seen. The review suggested discarding the
   record on every readout change; not taken -- a readout switch is not a
   new scene, and the fine hold already survives one. The last block
   readout's record keeps advancing instead.

3. "The fade is set only by a scroll listener": not in practice. The canvas's
   resize observer fires at first layout with or without a frame, and a check
   written for the state before any frame or scroll passed against the
   previous build at 400px. The bar, the name and the chip are observed as
   well, for the case the canvas cannot see -- a chip appearing when a frame
   lands pushes the tabs and can tip a bar that fitted into one that does not
   -- and the bar is measured once at reveal for a browser with no
   ResizeObserver. The check stays as a guard on the initial state.
@widgetii
widgetii merged commit 05b25eb into main Sep 25, 2026
1 check passed
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.

1 participant