fix(connect): six post-merge bugs + README hero screenshots#9
Merged
Conversation
Self-review of #8 surfaced six runtime/UX bugs that the test suite didn't catch. Tests added for each. 1. handleJdbc had an empty `if (p.password) { /* comment */ }` block whose comment claimed to queue the password on the clipboard but did nothing. Removed the misleading code and updated the toast to be honest: "JDBC URL copied · use the row's Copy button for the password". 2. handleCommand copied the command and then copied the password, leaving the password on the clipboard while the toast said "command copied" — the user would paste the password instead of the command. Now copies the command alone; password lives on the per-row Copy button. 3. launchSshUrl assigned to `window.location.href`, which is unreliable in Chromium for custom-scheme URLs: when no handler is registered Chrome navigates the tab to an `ERR_UNKNOWN_URL_SCHEME` page, throwing the user out of the app. Switched to the synthetic-anchor-click pattern used by 1Password / Bitwarden — the click invokes the protocol handler if present and is a silent no-op otherwise. 4. The "Open RDP session" option was enabled for any credential with a host, then called `buildRdpFile({ ...item, protocol: "rdp" })` — which overrode the protocol but kept the credential's port. For a Postgres credential on port 5432 the resulting .rdp pointed at `host:5432`, which doesn't run RDP. The option is now gated through a new `canBuildRdp` helper that requires `effectiveProtocol === "rdp"`, and the disabled-state hint guides the user to set the protocol field. 5. VaultPage.toggleSelectAll compared `s.size === filtered.length` to decide between select-all and deselect-all. With cross-scope selection that comparison can be coincidentally true even when the visible rows aren't the selected ones, causing a deselect when the user expected a select. Switched to a per-row `every`-based check that matches what CredentialsGrid uses for the header-checkbox visual. 6. The Connect dialog's "where this points" subtitle ran a dedupe pass keyed on exact string equality — but "db-prod-01" and "db-prod-01:5432" are different strings, so the host appeared twice in the rendered header (visible in the v0 vault-connect screenshot). Extracted the logic into a tested `buildTargetSubtitle` helper that keeps host:port as the canonical segment and only adds the IP separately when the hostname was used as the primary identifier. Tests: 48 passing (was 37) — +11 covering buildTargetSubtitle's dedupe and canBuildRdp's gating, including the regression case where a Postgres credential on port 5432 must NOT enable the RDP option. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The README's "this is what it does" was an ASCII architecture diagram — informative but it doesn't communicate the product. Added two screenshots above the diagram: 1. The vault dashboard (sidebar groupings, engine-coloured rows, Connect button per row) — the at-a-glance "what does Passman look like?" answer for someone landing on the repo. 2. The Connect dialog with JDBC / SSH / copy-command / RDP options — the headline feature that distinguishes this from a generic password manager. Both images already exist under docs/img/ and are kept in sync with the live styles via the docs/preview/ mockups, so README screenshots track the actual product without a separate maintenance burden. Tightened the description's first line to mention the DBA / infrastructure focus that the screenshots make obvious. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Two follow-up commits to #8 that arrived after the squash-merge so didn't make it into main:
fix(connect)— six bugs surfaced by post-merge self-review (one functional, two UX, one visual, one race, one wrong-condition).docs(readme)— adds two hero screenshots above the architecture diagram so a visitor scrolling the repo sees the product before the diagram.Bugs fixed
handleJdbchad an emptyif (p.password) { /* comment */ }block whose comment claimed to queue the password — code did nothinghandleCommandcopied the command and then overwrote it with the password — toast said "command copied" but clipboard held the passwordlaunchSshUrlusedwindow.location.href— Chromium navigates toERR_UNKNOWN_URL_SCHEMEif no handler is registered, throwing the user out of the app. Switched to the synthetic-anchor-click pattern used by 1Password / BitwardenbuildRdpFile({ ...item, protocol: "rdp" })overrode protocol but kept the port, generating an.rdpfile pointing athost:5432(RDP doesn't run on 5432). Now gated through a newcanBuildRdphelper that requireseffectiveProtocol === "rdp"toggleSelectAllcompareds.size === filtered.length— coincidental size match across scope changes flipped the toggle wrong direction. Switched tofiltered.every(it => s.has(it.id))matching the header-checkbox visualdb-prod-01anddb-prod-01:5432are different strings, so the host appeared twice in the rendered header. ExtractedbuildTargetSubtitlehelper with smart dedupeREADME hero
Two screenshots between the description and the ASCII architecture diagram:
The screenshots already exist under
docs/img/and stay in sync with the live styles via thedocs/preview/mockups, so this isn't a maintenance trap.Tests
buildTargetSubtitlededupe behaviour (host:port + non-redundant IP + AD-domain user encoding)canBuildRdpgating, including the explicit case that a Postgres credential on port 5432 must NOT enable the RDP option🤖 Generated with Claude Code