Skip to content

Fix/UI polish#7

Merged
psimaker merged 4 commits into
mainfrom
fix/ui-polish
May 17, 2026
Merged

Fix/UI polish#7
psimaker merged 4 commits into
mainfrom
fix/ui-polish

Conversation

@psimaker

@psimaker psimaker commented May 17, 2026

Copy link
Copy Markdown
Owner

Closes #6.

Summary

Two related fixes bundled together — a reported tap-collision bug on the home screen, plus the surrounding UI cleanup and translation gaps that surfaced while testing the fix.

Bug fix — conflict button no longer also opens Safari (#6)

SyncIssuesView placed a Link("Learn how to fix") and a NavigationLink("Resolve Conflicts") in the same List row. In SwiftUI, a NavigationLink makes the entire row tappable, so any tap — even directly on the link — fired both gestures: navigation and browser open.

  • New ExternalLinkButton helper: Button + openURL + .buttonStyle(.borderless) consumes taps and prevents the row-wide gesture from co-firing. Adds an arrow.up.right glyph to signal external destinations and a contentShape + vertical padding for hit-slop.
  • Rolled out to all 12 Link-style spots across ContentView, SettingsView, RelayDiagnosticsView, IgnorePatternsView for consistency.
  • Tightened .accessibilityElement(children: .combine) in SyncIssuesView so the link is its own VoiceOver target.
  • Unified Sync Issue action-button styles: critical = .borderedProminent, warnings = .bordered (previously pendingShares was an outlier).
  • Centralized 3 force-unwrapped doc URLs in a DocURL enum.

Home screen polish

  • Drop the "VaultSync" navigation title and keep the bar inline — reads as content, not chrome.
  • Devices list no longer prints the long Device ID under each name. The full ID stays one tap away in the device detail view.

Settings polish

  • Cloud Relay: hide per-device provisioning rows when the device is already .provisioned. Only devices that need attention surface.
  • Cloud Relay: remove the "Retry Provisioning" button. The same action remains available in Relay Diagnostics, where the advanced controls belong.
  • "This Device" section: remove the standalone Device ID display. The "Copy Device ID" button is enough on its own.

Translation gap

Reported case: the home-screen status Synchronisierungsprobleme showed "1 Conflict Needs Resolution" in English on a German locale. Root cause: the title strings for all four Sync Issue kinds were missing from every Localizable.strings, so NSLocalizedString was falling back to the key. Added 9 missing keys to en / de / zh-Hans:

  • 1 Vault Has Sync Errors / %d Vaults Have Sync Errors
  • At least one folder is currently in an error state.
  • 1 Required Device Is Disconnected / %d Required Devices Are Disconnected
  • 1 Pending Share Needs Attention / %d Pending Shares Need Attention
  • 1 Conflict Needs Resolution / %d Conflicts Need Resolution

Test plan

  • Existing test suite still passes (SubscriptionManagerTests still exercises retryRelayProvisioning against the manager directly — the method is unchanged, only the Settings UI entry point is removed).
  • Manual: drop a notes.sync-conflict-20260517-143022-ABCDEFG.md next to a real file in a vault → wait for next poll → home screen shows the Sync Issues section → tap Resolve Conflicts → ConflictListView opens without the browser also launching.
  • Manual: tap "Learn how to fix" in the same row → only the browser opens, no navigation.
  • Manual: switch device language to German → with at least one conflict, the title reads "1 Konflikt muss gelöst werden" / "%d Konflikte müssen gelöst werden". Same check for zh-Hans.
  • Manual: home screen no longer shows "VaultSync" title; device rows show only name + connected badge.
  • Manual: Settings → Cloud Relay shows no per-device row when the only paired device is Provisioned; the "Retry Provisioning" button is gone.
  • Manual: Settings → This Device shows only the "Copy Device ID" button (no inline ID block).

Fix: Prevent tap-collision bug and apply UI polish

This PR fixes issue #6, a tap-collision bug where tapping external documentation links in sync issue rows would also trigger the row's navigation action.

Main Fix

Introduced a new ExternalLinkButton component (a borderless button with an external link arrow that opens URLs via openURL, with proper hit-slop handling) to replace shared-row Link instances. Rolled out across 5 views:

  • ContentView (current sync error and per-vault folder error sections)
  • SyncIssuesView (per-issue "Learn how to fix" links)
  • RelayDiagnosticsView (relay troubleshooting links)
  • IgnorePatternsView (filter documentation link)
  • SettingsView (Cloud Relay troubleshooting and about links)

UI Polish

  • Removed "VaultSync" navigation title from home screen (kept inline bar display)
  • Removed full Device ID from device list rows (ID remains accessible in device detail view and via "Copy Device ID" button)
  • Simplified Cloud Relay provisioning UI: hide per-device rows when already .provisioned; removed standalone "Retry Provisioning" button from Settings (action remains in Relay Diagnostics)
  • Centralized force-unwrapped doc URLs into a DocURL enum; updated Privacy Policy and Terms of Use links to use it

Localization

Added 9 missing localization keys for Sync Issue titles and folder-error messages across English, German, and Simplified Chinese (en/de/zh-Hans), including singular/plural variants for conflicts, pending shares, disconnected devices, and sync errors.

Test Coverage

Existing test suite passes. Manual UI test steps documented in PR.

Review Change Stack

psimaker added 2 commits May 17, 2026 23:06
The "Resolve Conflicts" NavigationLink in the home-screen Sync Issues
section shared a List row with a "Learn how to fix" Link, so a tap on
either control fired both gestures — opening Safari and pushing the
conflict list at the same time.

Replace every "Learn how to fix" Link with a new ExternalLinkButton
helper (Button + openURL + .buttonStyle(.borderless)) that consumes
taps and prevents the row-wide NavigationLink gesture from co-firing.
The helper adds an arrow.up.right glyph to mark external destinations
and increases the hit-slop via contentShape + vertical padding.

Also fixed in SyncIssuesView while in the area:
- Action-button style unified: critical issues stay borderedProminent,
  warnings (including pendingShares) are now consistently bordered.
- accessibilityElement(children: .combine) tightened to wrap only the
  text trio, so the link button is its own VoiceOver target.

Centralized the three static documentation URLs in a DocURL enum to
remove force-unwrapped URL literals from view code.
Home screen:
- Drop the "VaultSync" navigation title and keep the bar inline, so the
  top of the list reads as content instead of chrome.
- Devices list no longer shows the long Device ID under each name; the
  full ID stays one tap away in the device detail view.

Settings → Cloud Relay:
- Hide the per-device provisioning row when the device is already
  provisioned, so only devices that actually need attention surface.
- Remove the "Retry Provisioning" button (the same action remains
  available from Relay Diagnostics, where advanced controls belong).

Settings → This Device:
- Remove the standalone Device ID display; the "Copy Device ID" button
  is sufficient on its own.

Localization:
- Add the missing Sync Issue titles and the folder-error message to
  en/de/zh-Hans Localizable.strings. The reported case
  "1 Conflict Needs Resolution" showed up untranslated; the other three
  issue kinds had the same gap and are fixed in the same pass.
@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@psimaker has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 3 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 70a05420-ac5e-4673-a676-db9fced1d473

📥 Commits

Reviewing files that changed from the base of the PR and between 531bfb6 and 9d2ccdb.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • README.md
  • ios/project.yml
📝 Walkthrough

Walkthrough

This PR consolidates external link handling by introducing ExternalLinkButton component and DocURL constants, replaces hardcoded Link views across multiple screens with the new button component, refactors Settings device information UI to use copy-on-tap for device ID, removes retry provisioning flow state, and adds multi-language sync issue titles.

Changes

ExternalLinkButton Refactor & Settings UX Polish

Layer / File(s) Summary
ExternalLinkButton Component & DocURL Constants
ios/VaultSync/Views/ExternalLinkButton.swift
New borderless SwiftUI button component that opens URLs via SwiftUI's openURL environment action, avoiding unintended list row navigation. Includes DocURL enum with static privacyPolicy, termsOfUse, and syncthingIgnoring URL constants.
Troubleshooting Link Migration Across Views
ios/VaultSync/Views/ContentView.swift, ios/VaultSync/Views/RelayDiagnosticsView.swift, ios/VaultSync/Views/SettingsView.swift, ios/VaultSync/Views/IgnorePatternsView.swift, ios/VaultSync/Views/SyncIssuesView.swift
Replaces 12 hardcoded Link implementations with ExternalLinkButton(titleKey:url:) across troubleshooting surfaces: current sync errors, per-folder errors, Obsidian access issues, relay diagnostics (five locations), and sync issues detail panel. Ensures consistent external link behavior and prevents list row tap conflicts.
Settings About Section Link Centralization
ios/VaultSync/Views/SettingsView.swift
Updates About section Privacy Policy and Terms of Use links from hardcoded GitHub markdown URLs to DocURL.privacyPolicy and DocURL.termsOfUse for centralized URL management.
Device ID Copy Button UI
ios/VaultSync/Views/SettingsView.swift
Replaces labeled Device ID text stack display with a "Copy Device ID" button interaction in the This Device section.
Cloud Relay Provisioning & State Cleanup
ios/VaultSync/Views/SettingsView.swift
Removes retryProvisioningInProgress state variable and associated "Retry Provisioning" button UI; refactors per-device provisioning row to show simplified status block for non-provisioned devices with failure reasons as secondary text and troubleshooting via ExternalLinkButton.
Navigation & Device List Refinements
ios/VaultSync/Views/ContentView.swift
Changes NavigationStack title from "VaultSync" to empty string; removes device ID secondary text from device list rows, leaving only device name display.
Button Style Adjustment
ios/VaultSync/Views/SyncIssuesView.swift
Changes pending shares action button style from .borderedProminent to .bordered.
Multi-Language Sync Issue Titles
ios/VaultSync/en.lproj/Localizable.strings, ios/VaultSync/de.lproj/Localizable.strings, ios/VaultSync/zh-Hans.lproj/Localizable.strings
Adds new "Sync Issue titles & messages" localization section with singular/plural variants for vault sync errors, disconnected required devices, pending shares needing attention, and conflicts requiring resolution across English, German, and Simplified Chinese.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • psimaker/vaultsync#2: This PR's IgnorePatternsView link consolidation directly extends the sync filters feature introduced in that PR by replacing its hardcoded documentation link with the new ExternalLinkButton component.

Poem

A button was born to escape the list,
With URLs safe from the tap's twisted fist,
Links now consolidated, device ID copied with care,
Settings breathe simpler—clean UI everywhere,
Copy and click, no confusion to share! 📋✨

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive Title 'Fix/UI polish' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes. Use a conventional-commit style title such as 'fix: prevent external link tap collision in sync issues row' or 'fix(ui): external link button and localization updates' for clarity.
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed PR fully addresses issue #6 by introducing ExternalLinkButton to prevent tap-collision bugs between row navigation and external link actions.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue #6 or directly related UI polish discovered during testing; no unrelated features or scope creep detected.
No Private Note Leakage ✅ Passed No privacy risks found. ExternalLinkButton uses static URLs only. Device IDs removed from home screen. No logging, analytics, or network operations expose sensitive data.
Bounded Ios Background Work ✅ Passed This PR contains no iOS background execution work. Changes are UI-only: component refactoring (ExternalLinkButton), view polish, and localization. Check is not applicable to these changes.
Bridge Contract Compatibility ✅ Passed PR contains no Go bridge or Swift bridge-service changes. All modifications are in iOS view layer and localization files. Bridge contract remains untouched and compatible.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ui-polish

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

psimaker added 2 commits May 17, 2026 23:22
Patch release covering the conflict-button gesture-collision fix (#6),
home and settings decluttering, and the missing en/de/zh-Hans
translations for the Sync Issue titles.
CHANGELOG entry for 1.3.1 covering the conflict-button gesture
collision fix (#6), home-screen and settings decluttering, and the
missing German/Simplified Chinese translations for the Sync Issue
titles. README "What's New" updated to match.
@psimaker psimaker merged commit 4af820f into main May 17, 2026
6 checks passed
@psimaker psimaker deleted the fix/ui-polish branch May 17, 2026 21:39
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.

Conflict button triggers browser every time

1 participant