fix(connection-dialog): prevent number input default value override on clear - #62
Merged
GOODBOY008 merged 1 commit intoJul 26, 2026
Conversation
…n clear
- Add displayValues state to track number input values as number | ''
so React controlled inputs can render empty when user clears the field
- Introduce handleNumberInput() to decouple display values from
ConnectionConfig's strict number types
- Fix port, proxyPort, keepAliveInterval, serverAliveCountMax inputs
that would immediately snap back to their defaults when cleared
Before: onChange used || fallback (parseInt('') || 22 = 22)
After: empty input sets display to ''; valid numbers update config
via onValid callback; defaults still apply at connect time
Test: 494 tests pass, 0 new failures
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a UX issue in ConnectionDialog where clearing controlled number inputs (e.g., port/proxy port) immediately reverts to a default value, by separating the displayed value from the strictly-typed numeric config.
Changes:
- Introduces a
displayValuesstate to allow number inputs to render as empty ('') while editing without overwritingconfig. - Adds a shared
handleNumberInput()helper and routes the affected numeric inputs through it. - Ensures protocol switching updates both
config.portand the displayed port value.
Comment on lines
+112
to
+126
| const handleNumberInput = ( | ||
| field: keyof typeof initialDisplayValues, | ||
| rawValue: string, | ||
| onValid: (n: number) => void, | ||
| ) => { | ||
| if (rawValue === '') { | ||
| setDisplayValues(prev => ({ ...prev, [field]: '' })); | ||
| return; | ||
| } | ||
| const parsed = parseInt(rawValue, 10); | ||
| if (!Number.isNaN(parsed)) { | ||
| setDisplayValues(prev => ({ ...prev, [field]: parsed })); | ||
| onValid(parsed); | ||
| } | ||
| }; |
Comment on lines
+103
to
+108
| const initialDisplayValues = { | ||
| port: 22 as number | '', | ||
| proxyPort: 8080 as number | '', | ||
| keepAliveInterval: 60 as number | '', | ||
| serverAliveCountMax: 3 as number | '', | ||
| }; |
Comment on lines
+163
to
+168
| syncDisplayValues({ | ||
| port: editingConnection.port ?? 22, | ||
| proxyPort: editingConnection.proxyPort ?? 8080, | ||
| keepAliveInterval: editingConnection.keepAliveInterval ?? 60, | ||
| serverAliveCountMax: editingConnection.serverAliveCountMax ?? 3, | ||
| }); |
GOODBOY008
approved these changes
Jul 26, 2026
GOODBOY008
left a comment
GOODBOY008
left a comment
Owner
There was a problem hiding this comment.
@sunxiaobin89 Great catch! LGTM~
Copilot AI
added a commit
that referenced
this pull request
Jul 26, 2026
PR #61: - Add aria-label and aria-expanded to folder chevron button for accessibility - Restore activeConnectionIds memoization to prevent unnecessary re-renders - Fix expand/collapse state preservation using flat map for deep tree support PR #62: - Replace parseInt with Number() + integer floor for number inputs - Reset to protocol-specific default port when field is cleared - Use getDefaultPort(protocol) for port fallback when editing connections
Copilot AI
added a commit
that referenced
this pull request
Jul 26, 2026
…icts Resolved conflicts in: - src/App.tsx: kept typeof guard for folderPath parameter - src/__tests__/connection-dialog-folder.test.tsx: kept proper ComponentProps import - src/components/connection-dialog.tsx: kept main's displayValues/handleNumberInput pattern from PR #62, re-applied dedicated initialFolder effect fix - src/components/connection-manager.tsx: kept main's mergeExpanded from PR #61, re-applied conditional onNewConnection rendering and aria-label fixes
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
When clearing a number input field in the connection dialog (port, proxy port, etc.), the value immediately snaps back to its default (22, 8080, etc.) instead of allowing the user to type a new value.
Root Cause
In handlers using the pattern
parseInt(e.target.value) || 22, clearing the field producesNaN || 22 = 22, whichupdateConfig()immediately writes back — React controlled input then renders22.Changes
displayValuesstate object (number | '') that tracks what each number input should visually display, separate fromConnectionConfig's strictnumbertype. Introduced a generichandleNumberInput()helper to handle all 4 affected inputs (port, proxyPort, keepAliveInterval, serverAliveCountMax) with the same logic.Testing