fix(ui): restore titlebar dragging on the right, and unbreak AeroSync template export - #516
Conversation
… template export The titlebar's right half could not drag or double-click the window. The reserved 96px navigation slot and the inter-cluster gaps carried no drag region, and on My Servers and Add Service that slot renders no button at all, leaving an inert strip immediately left of AeroVault. Fill the reserved width and the two cluster gaps with explicit drag regions; the outer container stays free of one, since a drag region spanning the interactive controls is what used to swallow clicks on Windows. Every AeroSync template export failed with "Profile '<id>' not found", in all four formats. The dialog was handed the connected saved-server id, while the three export commands resolve that id against the sync-preset store (Mirror, Two-way, Backup, Pull, Remote Backup), so the lookup could never match. Add a preset picker to the dialog and pass the saved server separately, as the name the generated script connects with. That second half was wrong on its own: the exported script wrote CONNECT --profile with the template name the user typed, which the CLI cannot resolve against the vault, so even a script that had exported would not have run. The path-bar chevron now points down while its dropdown is open, matching File Explorer, and the separator chevron gained the aria-haspopup and aria-expanded its sibling already had. Reported in #511 and #514, and in discussion #347. Co-Authored-By: Ehud Kirsh <100953560+EhudKirsh@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe sync template dialog now selects backend-loaded sync presets for exports while using the active saved server name for connection metadata. Breadcrumb chevrons expose and visualize open state, and the custom titlebar adds explicit draggable spacer regions. ChangesSync preset export
Breadcrumb dropdown state
Titlebar drag regions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant AeroSyncDialog
participant SyncTemplateDialog
participant Backend
participant ExportScript
App->>AeroSyncDialog: provide activeProfileName
AeroSyncDialog->>SyncTemplateDialog: pass serverProfileName
SyncTemplateDialog->>Backend: load_sync_profiles_cmd
Backend-->>SyncTemplateDialog: return SyncProfile entries
SyncTemplateDialog->>Backend: export with presetId
Backend-->>ExportScript: generate script using serverProfileName
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/BreadcrumbBar.tsx`:
- Around line 411-425: Update the separator toggle in BreadcrumbBar so its
aria-haspopup value matches the rendered role-less dropdown; remove the
menu-specific declaration unless the dropdown is also converted to a complete
menu with menuitem roles and keyboard behavior. Preserve the existing
aria-expanded state and popup interaction.
In `@src/components/Sync/SyncTemplateDialog.tsx`:
- Around line 498-510: Give the preset select in the sync preset section an
accessible programmatic name by converting the visible syncPresets.title element
into a label associated with the control via matching htmlFor and id attributes,
or by adding an equivalent aria-label directly to the select.
- Around line 99-116: Clear syncProfiles and presetId at the start of the isOpen
branch in the useEffect before invoking load_sync_profiles_cmd, so every dialog
open begins with empty state and load failures cannot retain stale selections.
Keep the successful profile loading and preset selection behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b97577f4-8123-44ef-911e-fb5007022ee4
📒 Files selected for processing (6)
src/App.tsxsrc/components/AeroSync/AeroSyncDialog.tsxsrc/components/AeroSync/types.tssrc/components/BreadcrumbBar.tsxsrc/components/CustomTitlebar.tsxsrc/components/Sync/SyncTemplateDialog.tsx
…a menu the dropdown is not Addresses the CodeRabbit review on #516. The preset id and the loaded list survived a close and reopen, so a failed reload left the previous selection in place with Export still enabled. The id then reached the backend and produced the same "Profile not found" this change exists to remove. Both are now cleared before the load rather than only on success, which also drops the id-preserving branch: the picker settles on the first preset, and there is no path where a removed custom preset stays selected. The chevron buttons no longer claim aria-haspopup="menu". Their dropdown is a plain list of buttons with no role="menu" and no arrow-key navigation, so the declaration promised keyboard behaviour that does not exist. Removed from the trailing chevron as well, where it predates this branch, rather than leaving the two siblings inconsistent. aria-expanded stays: on a disclosure button it is accurate on its own. The preset select gained a real label element tied to it by id, so assistive technology announces a named control instead of reading a nearby div. Co-Authored-By: Ehud Kirsh <100953560+EhudKirsh@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All three findings applied in On the ARIA popup semantics: correct, and the inconsistency was not introduced here. The trailing chevron already declared The select now has a Re-verified after the change: |
1 participant
Three reports from Ehud Kirsh, one commit: two window-level defects in the titlebar and the AeroSync template export, plus the path-bar chevron from the wishlist.
The titlebar could not be dragged from its right half (#511)
The outer titlebar container deliberately carries no
data-tauri-drag-region: on Windows the runtime intercepts the mousedown geometrically over a drag region before the DOM event is dispatched, which used to swallow clicks on modal close buttons falling inside the bar's 36px. The affordance was left to two explicit children, and everything to the right of the AeroShare button was neither.Two dead zones, both now filled with drag regions that contain no interactive children, so the Windows behaviour above cannot come back:
gap-3on the flex container. A gap is dead space the window manager cannot see, so they are now explicit spacers of the same width.This also restores double-click to maximise and restore on that side, which follows the drag region rather than a handler of ours.
AeroSync template export failed in all four formats (#514)
Export returned
Profile '<id>' not foundfor.aeroftp-script,.aerosync, bash and PowerShell alike. Two objects in this codebase are called a profile, and the dialog was handed the wrong one: the connected saved server, while all three export commands resolve the id against the sync preset store (Mirror, Two-way, Backup, Pull, Remote Backup). The lookup could never match, so no format could ever have worked.The dialog now has a preset picker, which is the object the export actually describes (direction, comparison keys, retry policy), and the saved server travels separately as what the generated script connects with.
That second half was a defect on its own, latent behind the first. The export wrote
CONNECT --profilewith the template name the user typed into the dialog, and the bash and PowerShell wrappers put the same value inaeroftp-cli sync --profile. Since the CLI resolves that name against the vault, even a script that had exported successfully would not have connected. The string in the error message is the saved-server id, not a credential.The path-bar chevron now points down while its menu is open (discussion #347)
Pure CSS,
rotate-90on the open state, matching what File Explorer does. One component covers every surface Ehud named:BreadcrumbBaris shared by AeroFile, the local panel and the remote panel. The separator chevrons also gained thearia-haspopupandaria-expandedthe trailing one already had.Verification
tsc(TS 7.0.2) clean,npm run buildok,vitest531/531,cargo clippy --all-features --testsexit 0,i18n:validate46/46 clean with 0 placeholders. No new i18n key: the preset picker reusessyncPresets.title, which is already translated in all 47 locales, so this PR does not touch the locale files and cannot conflict with the i18n chain.Frontend only. Branched off
mainat64efc8f5, after TypeScript 7.Merge order
This is outside the agreed
#502 -> #506 -> #509chain and blocks nobody, but it shares one file with #506 (src/App.tsx, one added line in a different block), so it should land after it.Reported in #511 and #514, and in discussion #347.
Not marked as closing either issue: they stay open until Ehud confirms on Windows 10.
Summary by CodeRabbit
New Features
Bug Fixes