fix: right-align config values, and drop the search screen the host already provides - #104
Merged
Merged
Conversation
…lready provides Two corrections after reading OpenCode's own dialog-select. Every ui.dialog.select already ships a live fuzzy filter over title and category, weighted 2:1 toward title, with a Search input drawn at the top of each dialog. The installed plugin API exposes neither skipFilter nor renderFilter, so the filter cannot be turned off — it is always there. The search screen added in 2.14.0 was therefore redundant, and worse than the host's: it required pressing Enter into a prompt before a single character could be typed, and its matching was not live. Removed it, with no replacement affordance, since the filter is already on every list. Values move to `footer`, which the host renders right-aligned against a flexing title. They used to sit in the description immediately after the label, so every row's value began at a different column and the eye had to read nine rows to compare two settings. The same nine values now line up in a column of their own. One rule across all three screens: title names the row, description disambiguates it, footer reports its current state. The restyle exposed a bug. The dirty marker was driven by the adapter's whole-draft configIsDirty, so staging one field marked every row in the block. Harmless in a description, actively harmful in a right-aligned column — that column is precisely what a user scans to find their own edit, and it would have lied on every untouched row. configFieldDirty marks the row and only the row. `disabled` rows are filtered out of the host's list rather than dimmed, so a field with no editor is absent, not faint, and a block claiming "3 settings" while showing two was a lie. The uneditable count moves to the block's description, the one row that survives the filter. Zero for every block shipped today. `current` also moves the cursor, not just the marker, so it is set on the hub's scope row only; on a per-block field list it would park the cursor on whichever field happened to be current and skip the first. `category` grouping is left unused: a field list is per-block and its title already names the block, so a category header would repeat it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two corrections after reading OpenCode's dialog-select source
I checked the host implementation (
packages/tui/src/ui/dialog-select.tsx) rather than only the plugin type definitions, and found I had reasoned from the wrong file last release.1. The search screen was redundant, and worse than what the host does
Every
ui.dialog.selectalready renders a live fuzzy filter overtitleandcategory, weighted 2:1 toward title, with aSearchinput drawn at the top of each dialog. The installed plugin API exposes neitherskipFilternorrenderFilter, so it cannot be turned off.The search screen added in 2.14.0 was therefore redundant — and worse than the host's: it required pressing Enter into a
promptbefore a single character could be typed, and its matching was not live. Removed, with no replacement affordance, since the filter is already on every list.The
config flow searchdescribe (8 tests) is deleted, not weakened: there is no remaining our behaviour to test. A guard now asserts the adapter never passesskipFilter/renderFilter.2. Values now right-align in
footerThe host renders
footerright-aligned against a flexing title. Values used to sit in the description immediately after the label, so every row's value began at a different column and comparing two settings meant reading nine rows.One rule across all three screens: title names the row, description disambiguates it, footer reports its current state.
Three things verified rather than assumed
currentalso moves the cursor — it draws●and callssetStore("selected", currentIndex). On a per-block field list it would park the cursor on whichever field happened to be current and skip the first row. Set on the hub's scope row only.categorygrouping is left unused. A field list is per-block and its title already readsNexus configuration — budget, so a bold accent header would repeat it. Left documented atfieldOptionsrather than added as a redundant header.disabledrows are filtered out, not dimmed.filtered()dropsx.disabled !== truebefore anything is drawn, so a field with no editor is absent. A block claiming "3 settings" while showing two was a lie; the uneditable count now moves to the block's description, the one row that survives the filter. Zero for every block shipped today.A bug the restyle exposed
The dirty marker was driven by the adapter's whole-draft
configIsDirty, so staging one field marked every row in the block. In a description that was merely redundant; in a right-aligned column it is actively harmful — that column is exactly what a user scans to find their own edit, and it lied on every untouched row.configFieldDirtynow marks the row and only the row, pinned by a test asserting exactly one of three siblings is marked.Verified
bun test1385 pass / 0 fail,bunx tsc --noEmitclean,bun run lintclean (3 pre-existing infos),bun run buildclean.I also rendered the real rows against the real config and read them: staging
budget.maxTotalCostmarks that row and its block, leavesselfHealinguntouched, and flips Save fromNo changes to writetoWrite the staged changes to disk.Note
PR is a
fix:— it removes a feature added in 2.14.0 and corrects a bug it introduced. The line count is large because the search screen's removal and the row-routing rewrite overlap in the same functions.