git_ui: Close branch selector as soon as branch is selected (#39725)
I noticed that branch picker doesn't close until the checkout operation is completed. While normally it's not an issue, it becomes obvious if there are longer running post checkout hooks. In that case selecting a branch makes it feel like nothing has happened (there's a small indicator in the footer) so it's possible to click it multiple times. Closing the modal before the operation completes leads to the error modal saying `Failed to change branch. entity released. Please try again.` even though the checkout was successful. The new behavior is to close the branch picker as soon as the branch is selected. This also aligns with the existing behavior in `create_branch` where `cx.emit(DismissEvent);` is called without waiting for `repo.update`. And as I mentioned before there an indicator in the footer saying `git switch <branch_name>` with a spinner thingy. I also added a check in the picker's `open` function where it first checks if there's currently an active job and does not show the picker in that case. If this generally makes sense I can add the tests as well if needed. P.S I checked how it works in VSCode and yes it also closes the branch picker as soon as the branch is selected. The only difference is that they show the loading indicator right next to the branch name (with a new branch) but in our case the current branch and activity indicator are located in different places. <details><summary>Before</summary> https://github.com/user-attachments/assets/adf08967-d908-45fa-b3f6-96f73d321262 </details> <details><summary>After</summary> https://github.com/user-attachments/assets/88c7ca41-7b39-42d6-a98b-3ad19da9317c </details> Release Notes: - The branch picker now closes immediately after a branch is selected, instead of waiting for the branch switch to complete.
This commit is contained in:
@@ -137,13 +137,13 @@ impl BranchList {
|
||||
})
|
||||
.await;
|
||||
|
||||
this.update_in(cx, |this, window, cx| {
|
||||
let _ = this.update_in(cx, |this, window, cx| {
|
||||
this.picker.update(cx, |picker, cx| {
|
||||
picker.delegate.default_branch = default_branch;
|
||||
picker.delegate.all_branches = Some(all_branches);
|
||||
picker.refresh(window, cx);
|
||||
})
|
||||
})?;
|
||||
});
|
||||
|
||||
anyhow::Ok(())
|
||||
})
|
||||
@@ -410,37 +410,20 @@ impl PickerDelegate for BranchListDelegate {
|
||||
return;
|
||||
}
|
||||
|
||||
cx.spawn_in(window, {
|
||||
let branch = entry.branch.clone();
|
||||
async move |picker, cx| {
|
||||
let branch_change_task = picker.update(cx, |this, cx| {
|
||||
let repo = this
|
||||
.delegate
|
||||
.repo
|
||||
.as_ref()
|
||||
.context("No active repository")?
|
||||
.clone();
|
||||
let Some(repo) = self.repo.clone() else {
|
||||
return;
|
||||
};
|
||||
|
||||
let mut cx = cx.to_async();
|
||||
let branch = entry.branch.clone();
|
||||
cx.spawn(async move |_, cx| {
|
||||
repo.update(cx, |repo, _| repo.change_branch(branch.name().to_string()))?
|
||||
.await??;
|
||||
|
||||
anyhow::Ok(async move {
|
||||
repo.update(&mut cx, |repo, _| {
|
||||
repo.change_branch(branch.name().to_string())
|
||||
})?
|
||||
.await?
|
||||
})
|
||||
})??;
|
||||
|
||||
branch_change_task.await?;
|
||||
|
||||
picker.update(cx, |_, cx| {
|
||||
cx.emit(DismissEvent);
|
||||
|
||||
anyhow::Ok(())
|
||||
})
|
||||
}
|
||||
anyhow::Ok(())
|
||||
})
|
||||
.detach_and_prompt_err("Failed to change branch", window, cx, |_, _, _| None);
|
||||
|
||||
cx.emit(DismissEvent);
|
||||
}
|
||||
|
||||
fn dismissed(&mut self, _: &mut Window, cx: &mut Context<Picker<Self>>) {
|
||||
|
||||
Reference in New Issue
Block a user