Skip to content

feat(desktop): show a working indicator#12941

Open
foxfirecodes wants to merge 1 commit intogitbutlerapp:masterfrom
foxfirecodes:lew/working-indicator
Open

feat(desktop): show a working indicator#12941
foxfirecodes wants to merge 1 commit intogitbutlerapp:masterfrom
foxfirecodes:lew/working-indicator

Conversation

@foxfirecodes
Copy link
Contributor

🧢 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:

  • the positioning of the indicator in the bottom right is not my favorite thing. maybe it belongs in the header? it can sorta clash with toasts rn (it shows above the very bottom right corner of the toast)
  • the way we're wrapping invokeTauri in trackOperation feels 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 in trackOperation would 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

Copilot AI review requested due to automatic review settings March 19, 2026 21:57
@vercel
Copy link

vercel bot commented Mar 19, 2026

@foxfirecodes is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 tauriInvoke calls with an operation-tracking helper so all IPC calls contribute to a shared “active operations” count.
  • Introduce activeOperations.svelte.ts to 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.

Comment on lines +9 to +12
{#if indicator.visible}
<div class="working-indicator" title="Operation in progress...">
<div class="working-indicator__dot"></div>
</div>
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 199 to 212
<ShareIssueModal />
<ToastController />
<ChipToastContainer />
<AppUpdater />
<PromptModal />
<ZoomInOutMenuAction />
<GlobalSettingsMenuAction />
<ReloadMenuAction />
<SwitchThemeMenuAction />
<ToggleSidebarMenuAction />
<GlobalModal />
<FocusCursor />
<WorkingIndicator />

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
this.unsubscribe = () => {
listeners = listeners.filter((l) => l !== onCountChange);
};
if (activeCount > 0) {
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
if (activeCount > 0) {
if (activeCount > 0) {
this.visibleSince = performance.now();

Copilot uses AI. Check for mistakes.
Comment on lines +290 to +297
return trackOperation(
invokeTauri<T>(command, params).catch((error: unknown) => {
if (isReduxError(error)) {
console.error(`ipc->${command}: ${JSON.stringify(params)}`, error);
}
throw error;
}),
);
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@Byron
Copy link
Collaborator

Byron commented Mar 19, 2026

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 master, there was some hickup with generated types apparently that were fixed now.

@codex review

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +30 to +33
promise.finally(() => {
activeCount--;
scheduleFlush();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@foxfirecodes foxfirecodes force-pushed the lew/working-indicator branch from 7e5714b to 5fa52e5 Compare March 19, 2026 22:53
@vercel
Copy link

vercel bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
gitbutler-web Skipped Skipped Mar 19, 2026 10:53pm

Request Review

@PavelLaptev
Copy link
Contributor

@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

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.

4 participants