Fix various activity indicator issues (#56469)
Follow-up to #54791 with some fixes to the activity indicator which resulted in - the button no longer being clickable when it should - the indicator showing a spinner instead of the proper symbol when downloading or when it should show a warning symbol Release Notes: - Fixed some issues where the activity indicator would show the wrong icon and not be clickable on errors present.
This commit is contained in:
Finn Evers authored and GitHub committed
1 parent
bf423dfc45
commit
d35e1cdbbb
2 files changed
+121
-159
No files matched your search
@@ -22,7 +22,7 @@ use std::{
|
||||
sync::Arc,
|
||||
time::{Duration, Instant},
|
||||
};
|
||||
use ui::{CommonAnimationExt, ContextMenu, PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*};
|
||||
use ui::{ContextMenu, PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*};
|
||||
use util::truncate_and_trailoff;
|
||||
use workspace::{StatusItemView, Workspace, item::ItemHandle};
|
||||
|
||||
@@ -62,8 +62,13 @@ struct PendingWork<'a> {
|
||||
progress: &'a LanguageServerProgress,
|
||||
}
|
||||
|
||||
enum ActivityIcon {
|
||||
LoadingSpinner,
|
||||
Icon(IconName),
|
||||
}
|
||||
|
||||
struct Content {
|
||||
icon: Option<gpui::AnyElement>,
|
||||
icon: ActivityIcon,
|
||||
message: String,
|
||||
on_click:
|
||||
Option<Arc<dyn Fn(&mut ActivityIndicator, &mut Window, &mut Context<ActivityIndicator>)>>,
|
||||
@@ -310,24 +315,19 @@ impl ActivityIndicator {
|
||||
.read(cx)
|
||||
.language_server_statuses(cx)
|
||||
.rev()
|
||||
.filter_map(|(server_id, status)| {
|
||||
if status.pending_work.is_empty() {
|
||||
None
|
||||
} else {
|
||||
let mut pending_work = status
|
||||
.pending_work
|
||||
.iter()
|
||||
.map(|(progress_token, progress)| PendingWork {
|
||||
language_server_id: server_id,
|
||||
progress_token,
|
||||
progress,
|
||||
})
|
||||
.collect::<SmallVec<[_; 4]>>();
|
||||
pending_work.sort_by_key(|work| Reverse(work.progress.last_update_at));
|
||||
Some(pending_work)
|
||||
}
|
||||
.flat_map(|(server_id, status)| {
|
||||
let mut pending_work = status
|
||||
.pending_work
|
||||
.iter()
|
||||
.map(|(progress_token, progress)| PendingWork {
|
||||
language_server_id: server_id,
|
||||
progress_token,
|
||||
progress,
|
||||
})
|
||||
.collect::<SmallVec<[_; 4]>>();
|
||||
pending_work.sort_by_key(|work| Reverse(work.progress.last_update_at));
|
||||
pending_work
|
||||
})
|
||||
.flatten()
|
||||
}
|
||||
|
||||
fn pending_environment_error<'a>(&'a self, cx: &'a App) -> Option<&'a String> {
|
||||
@@ -338,11 +338,7 @@ impl ActivityIndicator {
|
||||
// Show if any direnv calls failed
|
||||
if let Some(message) = self.pending_environment_error(cx) {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Warning)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Warning),
|
||||
message: message.clone(),
|
||||
on_click: Some(Arc::new(move |this, window, cx| {
|
||||
this.project.update(cx, |project, cx| {
|
||||
@@ -379,12 +375,7 @@ impl ActivityIndicator {
|
||||
}
|
||||
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Small)
|
||||
.with_rotate_animation(2)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::LoadingSpinner,
|
||||
message,
|
||||
on_click: Some(Arc::new(Self::toggle_language_server_work_context_menu)),
|
||||
tooltip_message: None,
|
||||
@@ -401,12 +392,7 @@ impl ActivityIndicator {
|
||||
.find(|s| !s.read(cx).is_started())
|
||||
{
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Small)
|
||||
.with_rotate_animation(2)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::LoadingSpinner,
|
||||
message: format!("Debug: {}", session.read(cx).adapter()),
|
||||
tooltip_message: session.read(cx).label().map(|label| label.to_string()),
|
||||
on_click: None,
|
||||
@@ -424,12 +410,7 @@ impl ActivityIndicator {
|
||||
&& Instant::now() - job_info.start >= GIT_OPERATION_DELAY
|
||||
{
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Small)
|
||||
.with_rotate_animation(2)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::LoadingSpinner,
|
||||
message: job_info.message.into(),
|
||||
on_click: None,
|
||||
tooltip_message: None,
|
||||
@@ -440,12 +421,7 @@ impl ActivityIndicator {
|
||||
for fs_job in &self.fs_jobs {
|
||||
if Instant::now().duration_since(fs_job.start) >= GIT_OPERATION_DELAY {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::ArrowCircle)
|
||||
.size(IconSize::Small)
|
||||
.with_rotate_animation(2)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::LoadingSpinner,
|
||||
message: fs_job.message.clone().into(),
|
||||
on_click: None,
|
||||
tooltip_message: None,
|
||||
@@ -498,11 +474,7 @@ impl ActivityIndicator {
|
||||
|
||||
if !downloading.is_empty() {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Download)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Download),
|
||||
message: format!(
|
||||
"Downloading {}...",
|
||||
downloading.iter().map(|name| name.as_ref()).fold(
|
||||
@@ -527,11 +499,7 @@ impl ActivityIndicator {
|
||||
|
||||
if !checking_for_update.is_empty() {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Download)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Download),
|
||||
message: format!(
|
||||
"Checking for updates to {}...",
|
||||
checking_for_update.iter().map(|name| name.as_ref()).fold(
|
||||
@@ -556,11 +524,7 @@ impl ActivityIndicator {
|
||||
|
||||
if !failed.is_empty() {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Warning)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Warning),
|
||||
message: format!(
|
||||
"Failed to run {}. Click to show error.",
|
||||
failed
|
||||
@@ -584,11 +548,7 @@ impl ActivityIndicator {
|
||||
// Show any formatting failure
|
||||
if let Some(failure) = self.project.read(cx).last_formatting_failure(cx) {
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Warning)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Warning),
|
||||
message: format!("Formatting failed: {failure}. Click to see logs."),
|
||||
on_click: Some(Arc::new(|indicator, window, cx| {
|
||||
indicator.project.update(cx, |project, cx| {
|
||||
@@ -630,11 +590,7 @@ impl ActivityIndicator {
|
||||
};
|
||||
|
||||
return Some(Content {
|
||||
icon: Some(
|
||||
Icon::new(IconName::Warning)
|
||||
.size(IconSize::Small)
|
||||
.into_any_element(),
|
||||
),
|
||||
icon: ActivityIcon::Icon(IconName::Warning),
|
||||
message: final_message,
|
||||
tooltip_message,
|
||||
on_click: Some(Arc::new(move |activity_indicator, window, cx| {
|
||||
@@ -656,32 +612,23 @@ impl ActivityIndicator {
|
||||
&& let Some((extension_id, operation)) =
|
||||
extension_store.outstanding_operations().iter().next()
|
||||
{
|
||||
let (message, icon, rotate) = match operation {
|
||||
let (message, icon) = match operation {
|
||||
ExtensionOperation::Install => (
|
||||
format!("Installing {extension_id} extension…"),
|
||||
IconName::LoadCircle,
|
||||
true,
|
||||
ActivityIcon::LoadingSpinner,
|
||||
),
|
||||
ExtensionOperation::Upgrade => (
|
||||
format!("Updating {extension_id} extension…"),
|
||||
IconName::Download,
|
||||
false,
|
||||
ActivityIcon::Icon(IconName::Download),
|
||||
),
|
||||
ExtensionOperation::Remove => (
|
||||
format!("Removing {extension_id} extension…"),
|
||||
IconName::LoadCircle,
|
||||
true,
|
||||
ActivityIcon::LoadingSpinner,
|
||||
),
|
||||
};
|
||||
|
||||
return Some(Content {
|
||||
icon: Some(Icon::new(icon).size(IconSize::Small).map(|this| {
|
||||
if rotate {
|
||||
this.with_rotate_animation(3).into_any_element()
|
||||
} else {
|
||||
this.into_any_element()
|
||||
}
|
||||
})),
|
||||
icon,
|
||||
message,
|
||||
on_click: Some(Arc::new(|this, window, cx| {
|
||||
this.dismiss_message(&Default::default(), window, cx)
|
||||
@@ -718,6 +665,8 @@ impl Render for ActivityIndicator {
|
||||
let activity_indicator = cx.entity().downgrade();
|
||||
let truncate_content = content.message.len() > MAX_MESSAGE_LEN;
|
||||
|
||||
let has_click_handler = content.on_click.is_some();
|
||||
|
||||
result.gap_2().child(
|
||||
PopoverMenu::new("activity-indicator-popover")
|
||||
.trigger(
|
||||
@@ -729,7 +678,14 @@ impl Render for ActivityIndicator {
|
||||
}
|
||||
})
|
||||
.label_size(LabelSize::Small)
|
||||
.loading(content.icon.is_some())
|
||||
.map(|this| match content.icon {
|
||||
ActivityIcon::LoadingSpinner => this.loading(true),
|
||||
ActivityIcon::Icon(icon_name) => this.start_icon(
|
||||
Icon::new(icon_name)
|
||||
.size(IconSize::Small)
|
||||
.color(Color::Muted),
|
||||
),
|
||||
})
|
||||
.map(|button| {
|
||||
if truncate_content {
|
||||
button.tooltip(Tooltip::text(content.message))
|
||||
@@ -746,64 +702,66 @@ impl Render for ActivityIndicator {
|
||||
}),
|
||||
)
|
||||
.anchor(gpui::Anchor::BottomLeft)
|
||||
.menu(move |window, cx| {
|
||||
let strong_this = activity_indicator.upgrade()?;
|
||||
let mut has_work = false;
|
||||
let menu = ContextMenu::build(window, cx, |mut menu, _, cx| {
|
||||
for work in strong_this.read(cx).pending_language_server_work(cx) {
|
||||
has_work = true;
|
||||
let activity_indicator = activity_indicator.clone();
|
||||
let mut title = work
|
||||
.progress
|
||||
.title
|
||||
.clone()
|
||||
.unwrap_or(work.progress_token.to_string());
|
||||
.when(!has_click_handler, |this| {
|
||||
this.menu(move |window, cx| {
|
||||
let strong_this = activity_indicator.upgrade()?;
|
||||
let mut has_work = false;
|
||||
let menu = ContextMenu::build(window, cx, |mut menu, _, cx| {
|
||||
for work in strong_this.read(cx).pending_language_server_work(cx) {
|
||||
has_work = true;
|
||||
let activity_indicator = activity_indicator.clone();
|
||||
let mut title = work
|
||||
.progress
|
||||
.title
|
||||
.clone()
|
||||
.unwrap_or(work.progress_token.to_string());
|
||||
|
||||
if work.progress.is_cancellable {
|
||||
let language_server_id = work.language_server_id;
|
||||
let token = work.progress_token.clone();
|
||||
let title = SharedString::from(title);
|
||||
menu = menu.custom_entry(
|
||||
move |_, _| {
|
||||
h_flex()
|
||||
.w_full()
|
||||
.justify_between()
|
||||
.child(Label::new(title.clone()))
|
||||
.child(Icon::new(IconName::XCircle))
|
||||
.into_any_element()
|
||||
},
|
||||
move |_, cx| {
|
||||
let token = token.clone();
|
||||
activity_indicator
|
||||
.update(cx, |activity_indicator, cx| {
|
||||
activity_indicator.project.update(
|
||||
cx,
|
||||
|project, cx| {
|
||||
project.cancel_language_server_work(
|
||||
language_server_id,
|
||||
Some(token),
|
||||
cx,
|
||||
);
|
||||
},
|
||||
);
|
||||
activity_indicator.context_menu_handle.hide(cx);
|
||||
cx.notify();
|
||||
})
|
||||
.ok();
|
||||
},
|
||||
);
|
||||
} else {
|
||||
if let Some(progress_message) = work.progress.message.as_ref() {
|
||||
title.push_str(": ");
|
||||
title.push_str(progress_message);
|
||||
if work.progress.is_cancellable {
|
||||
let language_server_id = work.language_server_id;
|
||||
let token = work.progress_token.clone();
|
||||
let title = SharedString::from(title);
|
||||
menu = menu.custom_entry(
|
||||
move |_, _| {
|
||||
h_flex()
|
||||
.w_full()
|
||||
.justify_between()
|
||||
.child(Label::new(title.clone()))
|
||||
.child(Icon::new(IconName::XCircle))
|
||||
.into_any_element()
|
||||
},
|
||||
move |_, cx| {
|
||||
let token = token.clone();
|
||||
activity_indicator
|
||||
.update(cx, |activity_indicator, cx| {
|
||||
activity_indicator.project.update(
|
||||
cx,
|
||||
|project, cx| {
|
||||
project.cancel_language_server_work(
|
||||
language_server_id,
|
||||
Some(token),
|
||||
cx,
|
||||
);
|
||||
},
|
||||
);
|
||||
activity_indicator.context_menu_handle.hide(cx);
|
||||
cx.notify();
|
||||
})
|
||||
.ok();
|
||||
},
|
||||
);
|
||||
} else {
|
||||
if let Some(progress_message) = work.progress.message.as_ref() {
|
||||
title.push_str(": ");
|
||||
title.push_str(progress_message);
|
||||
}
|
||||
|
||||
menu = menu.label(title);
|
||||
}
|
||||
|
||||
menu = menu.label(title);
|
||||
}
|
||||
}
|
||||
menu
|
||||
});
|
||||
has_work.then_some(menu)
|
||||
menu
|
||||
});
|
||||
has_work.then_some(menu)
|
||||
})
|
||||
}),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -190,6 +190,7 @@ impl Button {
|
||||
/// Displays a rotating loading spinner in place of the `start_icon`.
|
||||
///
|
||||
/// When `loading` is `true`, any `start_icon` is ignored. and a rotating
|
||||
/// loading spinner is shown instead.
|
||||
pub fn loading(mut self, loading: bool) -> Self {
|
||||
self.loading = loading;
|
||||
self
|
||||
@@ -389,23 +390,26 @@ impl RenderOnce for Button {
|
||||
h_flex()
|
||||
.when(self.truncate, |this| this.min_w_0().overflow_hidden())
|
||||
.gap(DynamicSpacing::Base04.rems(cx))
|
||||
.when(self.loading, |this| {
|
||||
this.child(
|
||||
Icon::new(IconName::LoadCircle)
|
||||
.size(IconSize::Small)
|
||||
.color(Color::Muted)
|
||||
.with_rotate_animation(2),
|
||||
)
|
||||
})
|
||||
.when(!self.loading, |this| {
|
||||
this.when_some(self.start_icon, |this, icon| {
|
||||
this.child(if is_disabled {
|
||||
icon.color(Color::Disabled)
|
||||
} else {
|
||||
icon
|
||||
.when_else(
|
||||
self.loading,
|
||||
|this| {
|
||||
this.child(
|
||||
Icon::new(IconName::LoadCircle)
|
||||
.size(IconSize::Small)
|
||||
.color(Color::Muted)
|
||||
.with_rotate_animation(2),
|
||||
)
|
||||
},
|
||||
|this| {
|
||||
this.when_some(self.start_icon, |this, icon| {
|
||||
this.child(if is_disabled {
|
||||
icon.color(Color::Disabled)
|
||||
} else {
|
||||
icon
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
},
|
||||
)
|
||||
.child(
|
||||
h_flex()
|
||||
.when(self.truncate, |this| this.min_w_0().overflow_hidden())
|
||||
|
||||
Reference in new issue
Block a user