feat(desktop): show a working indicator#12941
feat(desktop): show a working indicator#12941foxfirecodes wants to merge 1 commit intogitbutlerapp:masterfrom
Conversation
|
@foxfirecodes is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Adds a global “work in progress” indicator to the Desktop app by tracking in-flight backend (Tauri IPC) operations and rendering a small persistent UI affordance in the root layout.
Changes:
- Add a global working indicator component to the desktop root layout.
- Wrap
tauriInvokecalls with an operation-tracking helper so all IPC calls contribute to a shared “active operations” count. - Introduce
activeOperations.svelte.tsto track operation count and expose a reactive indicator with a minimum visible duration.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/desktop/src/routes/+layout.svelte | Mounts the global working indicator in the app shell. |
| apps/desktop/src/lib/backend/tauri.ts | Wraps Tauri IPC invocations in operation tracking. |
| apps/desktop/src/lib/activeOperations.svelte.ts | New shared tracker + reactive indicator model with min visible time. |
| apps/desktop/src/components/WorkingIndicator.svelte | New UI component showing a pulsing dot when operations are active. |
| {#if indicator.visible} | ||
| <div class="working-indicator" title="Operation in progress..."> | ||
| <div class="working-indicator__dot"></div> | ||
| </div> |
There was a problem hiding this comment.
title="Operation in progress..." won’t actually show a tooltip here because the element has pointer-events: none;, so it can’t be hovered/focused. Either drop the title, or allow pointer events on the wrapper (while keeping the dot non-interactive), and/or provide an accessible alternative like role="status" + an aria-label/visually-hidden text so non-mouse users can discover the state.
| <ShareIssueModal /> | ||
| <ToastController /> | ||
| <ChipToastContainer /> | ||
| <AppUpdater /> | ||
| <PromptModal /> | ||
| <ZoomInOutMenuAction /> | ||
| <GlobalSettingsMenuAction /> | ||
| <ReloadMenuAction /> | ||
| <SwitchThemeMenuAction /> | ||
| <ToggleSidebarMenuAction /> | ||
| <GlobalModal /> | ||
| <FocusCursor /> | ||
| <WorkingIndicator /> | ||
|
|
There was a problem hiding this comment.
The indicator uses z-index: var(--z-blocker) and is rendered after ToastController in +layout.svelte, while the toast container also uses z-index: var(--z-blocker) and is anchored bottom-right. With equal z-index, DOM order makes this indicator layer above toasts and can overlap/obscure toast UI; consider lowering the indicator z-index or rendering it earlier (or offsetting its position) to avoid covering toasts.
| this.unsubscribe = () => { | ||
| listeners = listeners.filter((l) => l !== onCountChange); | ||
| }; | ||
| if (activeCount > 0) { |
There was a problem hiding this comment.
ActiveOperationIndicator sets visible = true in the constructor when activeCount > 0, but it doesn’t initialize visibleSince. If the count then drops to 0, elapsed will be computed against 0, making remaining 0 and hiding immediately (skipping the intended minVisibleMs). Consider setting visibleSince = performance.now() when initializing visibility from an already-active operation.
| if (activeCount > 0) { | |
| if (activeCount > 0) { | |
| this.visibleSince = performance.now(); |
| return trackOperation( | ||
| invokeTauri<T>(command, params).catch((error: unknown) => { | ||
| if (isReduxError(error)) { | ||
| console.error(`ipc->${command}: ${JSON.stringify(params)}`, error); | ||
| } | ||
| throw error; | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This wraps tauriInvoke calls in trackOperation, but this file still has at least one direct invokeTauri(...) usage (tauriOpenExternalUrl) that won’t be tracked. If the goal is a global “any backend work” indicator, consider routing those remaining invokeTauri call sites through tauriInvoke (or otherwise tracking them) for consistency.
|
Thanks a lot for contributing! While I like the concept and also think it's useful, it's probably still something to flesh out design wise. Something I'd not want to have is 'popping' or layout shift. @PavelLaptev should probably take this over for that reason. I also recommend to rebase onto @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5714bcf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| promise.finally(() => { | ||
| activeCount--; | ||
| scheduleFlush(); | ||
| }); |
There was a problem hiding this comment.
Return the
finally chain instead of floating it
Please handle the promise created by finally here. When a Tauri command rejects, promise.finally(...) produces a second rejected promise that nobody awaits, so window.onunhandledrejection in apps/desktop/src/hooks.client.ts:63 still fires even if the caller catches the original error. A concrete case is UserService.getLoginUrl() (apps/desktop/src/lib/user/userService.ts:138), which intentionally swallows build_type failures; after this change that path will still surface an “Unhandled exception” toast and extra error logging.
Useful? React with 👍 / 👎.
7e5714b to
5fa52e5
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@foxfirecodes thanks for the contribution and for identifying a real pain point — the lack of feedback during operations is definitely something that needs to be addressed! However, I think a global indicator is masking the underlying problem rather than solving it. When a user clicks something, they need confirmation that their specific action registered and is being processed. A generic dot in the corner doesn't provide that reassurance. It also risks becoming background noise that users learn to ignore. The proper fix is contextual loading states: the button you clicked disables/shows a spinner, the list item you triggered gives visual feedback, etc. That's a bigger and harder effort, but it's the right direction. I'd suggest closing this PR for now. If you're interested in tackling the root problem, picking a few of the most-used interactions (like the dropdown actions you mentioned where we can show a progress bar ) and adding proper per-action loading states would be a great place to start. I've added GH desktop as an example: Screen.Recording.2026-03-20.at.00.02.02.mov |
🧢 Changes
this PR is a small UX improvement that i found myself wishing for constantly: show a small visual indicator whenever any operation is happening, somewhere that is readily visible.
☕️ Reasoning
while a lot of operations come with a dedicated loading state (like showing a spinner on a button), a lot of operations dont have any kind of interaction/loading indicator. which has lead to a number of times where i thought i clicked an item in a dropdown (like squash all commits) only to sit there staring waiting for it to finish... to then realize i didnt actually click it lmao
so by putting a persistent and consistent visual indicator that the app is working on something, it helps address confusion for interactions where its difficult to provide a good loading state.
📌 Todos
the only things i'm not 100% are:
invokeTauriintrackOperationfeels a little gross to me since it requires you to use promise methods instead of try/catch w/ async/await buuuut at the same time wrapping the entire async block intrackOperationwould nest the code a lot more. eh /shrug🎮 Demo
(pay attention to the bottom right corner of the window)
Screen.Recording.2026-03-19.at.17.54.24.mov