feat(writing): editable min/max for the custom Length range#653
Merged
Conversation
Replace the cramped side-by-side steppers with stacked Minimum and Maximum rows, each an editable numeric field paired with up/down arrows. Typed and stepped values both commit through setCustomWordCountRange, so they stay clamped to [1, 50] with Max >= Min. Users can now type a value directly instead of only nudging the arrows, and the layout reads more clearly.
Addresses Greptile feedback: LabeledContent's visible title isn't applied to the controls, so the TextField and Stepper were announced unnamed. Thread a label through wordCountField so Min and Max each get a distinct accessibility label.
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.
Summary
Makes the custom Length range directly editable. The old "Custom range…" UI was two side-by-side steppers with static
Min: N/Max: Nlabels, so values could only be nudged one at a time. This replaces them with stacked Minimum and Maximum rows, each a numeric text field paired with up/down arrows, so a user can type a value or step it. The sensible bounds are unchanged: both rows commit throughsetCustomWordCountRange, which clamps to[1, 50]and keeps Max >= Min.Validation
UI: rendered the real
WritingPaneViewin Custom-range mode via an offscreenNSHostingView(so the AppKit-backed field/stepper rasterize) and confirmed the layout: a "Minimum" row showing an editable5with arrows above a "Maximum" row showing48with arrows, then the existing token-budget caption. I did not launch the full menu-bar app, to avoid its accessibility / input-monitoring permission flow.Linked issues
None.
Risk / rollout notes
WritingPaneView.swift. No settings schema, persistence, orpbxprojchange.setCustomWordCountRange/SuggestionWordRange.clamped(low:high:)path, so typed values are bounded to[1, 50]with Max snapped up to Min on commit, exactly as the steppers were. The.numberfield commits on Return / focus loss, so the clamp never fights intermediate keystrokes.Greptile Summary
Replaces the old pair of side-by-side
Steppercontrols in the Custom Length UI with two stackedLabeledContentrows, each containing a numericTextFieldpaired with aSteppervia a sharedBinding<Int>. Users can now type a value directly or nudge it with arrows; all writes still route throughsetCustomWordCountRange, preserving the existing clamp-to-[1, 50]and Max ≥ Min invariants.wordCountField(value:label:)helper annotated@ViewBuilderthat renders theTextField+Stepperpair, applies.accessibilityLabelto both controls, and is reused for Minimum and Maximum rows.HStackof two rawStepperviews and replaces it with twoLabeledContentrows; the token-budget caption below is preserved in the same conditional block.Confidence Score: 5/5
Safe to merge — purely a UI-layer change in a single file, no schema, persistence, or model logic touched.
All value writes still funnel through the unchanged setCustomWordCountRange path, so the clamp-to-[1,50] and Max >= Min invariants are preserved for both typed and stepped input. The TextField with .number format defers binding updates until Return/focus loss, so intermediate keystrokes never fight the clamp. No test, migration, or configuration surface is affected.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User interaction] --> B{Input method} B -->|Type in TextField| C[Intermediate keystrokes buffered<br/>binding NOT called yet] B -->|Press Stepper arrow| D[Stepper calls binding.set<br/>immediately] C -->|Return / focus loss| E[TextField commits<br/>binding.set called] D --> F[customLowBinding or customHighBinding setter] E --> F F --> G[setCustomWordCountRange<br/>low:high:] G --> H{Clamp logic} H -->|low < minimumWord| I[low = minimumWord] H -->|high > maximumWord| J[high = maximumWord] H -->|high < low| K[high = low] I --> L[Persist to SuggestionSettingsModel] J --> L K --> L H -->|values in range| L L --> M[UI re-renders with clamped values]Reviews (2): Last reviewed commit: "fix(a11y): give the custom Min/Max word-..." | Re-trigger Greptile