Fix inactive tab styles (the verbose way) (#3591)

Marshall Bowers created

This PR fixes the inactive tab style to properly show the label using
the muted text color.

I went about fixing this in the most direct way possible, but the
solution leaves a lot to be desired, IMO. I plan to explore some ideas
on how we can improve the state of styling the tab content without
having the same styles repeated all over the place and subsequently
out-of-sync.

Release Notes:

- N/A

Change summary

crates/collab_ui2/src/channel_view.rs      | 12 +++-
crates/diagnostics2/src/diagnostics.rs     | 66 +++++++++++++----------
crates/editor2/src/items.rs                |  8 ++
crates/search2/src/project_search.rs       | 12 ++-
crates/terminal_view2/src/terminal_view.rs | 13 ++++
crates/welcome2/src/welcome.rs             | 17 ++++-
crates/workspace2/src/item.rs              | 11 ++-
crates/workspace2/src/pane.rs              |  8 +-
crates/workspace2/src/shared_screen.rs     | 20 +++++--
9 files changed, 108 insertions(+), 59 deletions(-)

Detailed changes

crates/collab_ui2/src/channel_view.rs 🔗

@@ -17,7 +17,7 @@ use std::{
     any::{Any, TypeId},
     sync::Arc,
 };
-use ui::Label;
+use ui::{prelude::*, Label};
 use util::ResultExt;
 use workspace::{
     item::{FollowableItem, Item, ItemEvent, ItemHandle},
@@ -253,7 +253,7 @@ impl Item for ChannelView {
         }
     }
 
-    fn tab_content(&self, _: Option<usize>, cx: &WindowContext) -> AnyElement {
+    fn tab_content(&self, _: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement {
         let label = if let Some(channel) = self.channel(cx) {
             match (
                 channel.can_edit_notes(),
@@ -266,7 +266,13 @@ impl Item for ChannelView {
         } else {
             format!("channel notes (disconnected)")
         };
-        Label::new(label).into_any_element()
+        Label::new(label)
+            .color(if selected {
+                Color::Default
+            } else {
+                Color::Muted
+            })
+            .into_any_element()
     }
 
     fn clone_on_split(&self, _: WorkspaceId, cx: &mut ViewContext<Self>) -> Option<View<Self>> {

crates/diagnostics2/src/diagnostics.rs 🔗

@@ -633,8 +633,44 @@ impl Item for ProjectDiagnosticsEditor {
         Some("Project Diagnostics".into())
     }
 
-    fn tab_content(&self, _detail: Option<usize>, _: &WindowContext) -> AnyElement {
-        render_summary(&self.summary)
+    fn tab_content(&self, _detail: Option<usize>, selected: bool, _: &WindowContext) -> AnyElement {
+        if self.summary.error_count == 0 && self.summary.warning_count == 0 {
+            let label = Label::new("No problems");
+            label.into_any_element()
+        } else {
+            h_stack()
+                .gap_1()
+                .when(self.summary.error_count > 0, |then| {
+                    then.child(
+                        h_stack()
+                            .gap_1()
+                            .child(IconElement::new(Icon::XCircle).color(Color::Error))
+                            .child(Label::new(self.summary.error_count.to_string()).color(
+                                if selected {
+                                    Color::Default
+                                } else {
+                                    Color::Muted
+                                },
+                            )),
+                    )
+                })
+                .when(self.summary.warning_count > 0, |then| {
+                    then.child(
+                        h_stack()
+                            .child(
+                                IconElement::new(Icon::ExclamationTriangle).color(Color::Warning),
+                            )
+                            .child(Label::new(self.summary.warning_count.to_string()).color(
+                                if selected {
+                                    Color::Default
+                                } else {
+                                    Color::Muted
+                                },
+                            )),
+                    )
+                })
+                .into_any_element()
+        }
     }
 
     fn for_each_project_item(
@@ -782,32 +818,6 @@ fn diagnostic_header_renderer(diagnostic: Diagnostic) -> RenderBlock {
     })
 }
 
-pub(crate) fn render_summary(summary: &DiagnosticSummary) -> AnyElement {
-    if summary.error_count == 0 && summary.warning_count == 0 {
-        let label = Label::new("No problems");
-        label.into_any_element()
-    } else {
-        h_stack()
-            .gap_1()
-            .when(summary.error_count > 0, |then| {
-                then.child(
-                    h_stack()
-                        .gap_1()
-                        .child(IconElement::new(Icon::XCircle).color(Color::Error))
-                        .child(Label::new(summary.error_count.to_string())),
-                )
-            })
-            .when(summary.warning_count > 0, |then| {
-                then.child(
-                    h_stack()
-                        .child(IconElement::new(Icon::ExclamationTriangle).color(Color::Warning))
-                        .child(Label::new(summary.warning_count.to_string())),
-                )
-            })
-            .into_any_element()
-    }
-}
-
 fn compare_diagnostics<L: language::ToOffset, R: language::ToOffset>(
     lhs: &DiagnosticEntry<L>,
     rhs: &DiagnosticEntry<R>,

crates/editor2/src/items.rs 🔗

@@ -580,7 +580,7 @@ impl Item for Editor {
         Some(path.to_string_lossy().to_string().into())
     }
 
-    fn tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement {
+    fn tab_content(&self, detail: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement {
         let theme = cx.theme();
 
         let description = detail.and_then(|detail| {
@@ -597,7 +597,11 @@ impl Item for Editor {
 
         h_stack()
             .gap_2()
-            .child(Label::new(self.title(cx).to_string()))
+            .child(Label::new(self.title(cx).to_string()).color(if selected {
+                Color::Default
+            } else {
+                Color::Muted
+            }))
             .when_some(description, |this, description| {
                 this.child(Label::new(description).color(Color::Muted))
             })

crates/search2/src/project_search.rs 🔗

@@ -33,8 +33,8 @@ use std::{
 };
 
 use ui::{
-    h_stack, v_stack, Button, ButtonCommon, Clickable, Disableable, Icon, IconButton, IconElement,
-    Label, LabelCommon, LabelSize, Selectable, Tooltip,
+    h_stack, prelude::*, v_stack, Button, Icon, IconButton, IconElement, Label, LabelCommon,
+    LabelSize, Selectable, Tooltip,
 };
 use util::{paths::PathMatcher, ResultExt as _};
 use workspace::{
@@ -511,7 +511,7 @@ impl Item for ProjectSearchView {
             .update(cx, |editor, cx| editor.deactivated(cx));
     }
 
-    fn tab_content(&self, _: Option<usize>, cx: &WindowContext<'_>) -> AnyElement {
+    fn tab_content(&self, _: Option<usize>, selected: bool, cx: &WindowContext<'_>) -> AnyElement {
         let last_query: Option<SharedString> = self
             .model
             .read(cx)
@@ -527,7 +527,11 @@ impl Item for ProjectSearchView {
             .unwrap_or_else(|| "Project search".into());
         h_stack()
             .child(IconElement::new(Icon::MagnifyingGlass))
-            .child(Label::new(tab_name))
+            .child(Label::new(tab_name).color(if selected {
+                Color::Default
+            } else {
+                Color::Muted
+            }))
             .into_any()
     }
 

crates/terminal_view2/src/terminal_view.rs 🔗

@@ -686,13 +686,22 @@ impl Item for TerminalView {
         Some(self.terminal().read(cx).title().into())
     }
 
-    fn tab_content(&self, _detail: Option<usize>, cx: &WindowContext) -> AnyElement {
+    fn tab_content(
+        &self,
+        _detail: Option<usize>,
+        selected: bool,
+        cx: &WindowContext,
+    ) -> AnyElement {
         let title = self.terminal().read(cx).title();
 
         h_stack()
             .gap_2()
             .child(IconElement::new(Icon::Terminal))
-            .child(Label::new(title))
+            .child(Label::new(title).color(if selected {
+                Color::Default
+            } else {
+                Color::Muted
+            }))
             .into_any()
     }
 

crates/welcome2/src/welcome.rs 🔗

@@ -3,12 +3,13 @@ mod base_keymap_setting;
 
 use db::kvp::KEY_VALUE_STORE;
 use gpui::{
-    div, red, AnyElement, AppContext, Div, Element, EventEmitter, FocusHandle, Focusable,
-    FocusableView, InteractiveElement, ParentElement, Render, Styled, Subscription, View,
-    ViewContext, VisualContext, WeakView, WindowContext,
+    div, red, AnyElement, AppContext, Div, EventEmitter, FocusHandle, Focusable, FocusableView,
+    InteractiveElement, ParentElement, Render, Styled, Subscription, View, ViewContext,
+    VisualContext, WeakView, WindowContext,
 };
 use settings::{Settings, SettingsStore};
 use std::sync::Arc;
+use ui::prelude::*;
 use workspace::{
     dock::DockPosition,
     item::{Item, ItemEvent},
@@ -261,8 +262,14 @@ impl FocusableView for WelcomePage {
 impl Item for WelcomePage {
     type Event = ItemEvent;
 
-    fn tab_content(&self, _: Option<usize>, _: &WindowContext) -> AnyElement {
-        "Welcome to Zed!".into_any()
+    fn tab_content(&self, _: Option<usize>, selected: bool, _: &WindowContext) -> AnyElement {
+        Label::new("Welcome to Zed!")
+            .color(if selected {
+                Color::Default
+            } else {
+                Color::Muted
+            })
+            .into_any_element()
     }
 
     fn show_toolbar(&self) -> bool {

crates/workspace2/src/item.rs 🔗

@@ -106,7 +106,7 @@ pub trait Item: FocusableView + EventEmitter<Self::Event> {
     fn tab_description(&self, _: usize, _: &AppContext) -> Option<SharedString> {
         None
     }
-    fn tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement;
+    fn tab_content(&self, detail: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement;
 
     /// (model id, Item)
     fn for_each_project_item(
@@ -218,7 +218,7 @@ pub trait ItemHandle: 'static + Send {
     fn focus_handle(&self, cx: &WindowContext) -> FocusHandle;
     fn tab_tooltip_text(&self, cx: &AppContext) -> Option<SharedString>;
     fn tab_description(&self, detail: usize, cx: &AppContext) -> Option<SharedString>;
-    fn tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement;
+    fn tab_content(&self, detail: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement;
     fn dragged_tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
     fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>;
@@ -311,12 +311,12 @@ impl<T: Item> ItemHandle for View<T> {
         self.read(cx).tab_description(detail, cx)
     }
 
-    fn tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement {
-        self.read(cx).tab_content(detail, cx)
+    fn tab_content(&self, detail: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement {
+        self.read(cx).tab_content(detail, selected, cx)
     }
 
     fn dragged_tab_content(&self, detail: Option<usize>, cx: &WindowContext) -> AnyElement {
-        self.read(cx).tab_content(detail, cx)
+        self.read(cx).tab_content(detail, true, cx)
     }
 
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath> {
@@ -941,6 +941,7 @@ pub mod test {
         fn tab_content(
             &self,
             detail: Option<usize>,
+            selected: bool,
             cx: &ui::prelude::WindowContext,
         ) -> AnyElement {
             self.tab_detail.set(detail);

crates/workspace2/src/pane.rs 🔗

@@ -1424,7 +1424,9 @@ impl Pane {
         detail: usize,
         cx: &mut ViewContext<'_, Pane>,
     ) -> impl IntoElement {
-        let label = item.tab_content(Some(detail), cx);
+        let is_active = ix == self.active_item_index;
+
+        let label = item.tab_content(Some(detail), is_active, cx);
         let close_side = &ItemSettings::get_global(cx).close_position;
 
         let (text_color, tab_bg, tab_hover_bg, tab_active_bg) = match ix == self.active_item_index {
@@ -1442,8 +1444,6 @@ impl Pane {
             ),
         };
 
-        let is_active = ix == self.active_item_index;
-
         let indicator = maybe!({
             let indicator_color = match (item.has_conflict(cx), item.is_dirty(cx)) {
                 (true, _) => Color::Warning,
@@ -1473,7 +1473,7 @@ impl Pane {
                     ClosePosition::Left => ui::TabCloseSide::Start,
                     ClosePosition::Right => ui::TabCloseSide::End,
                 })
-                .selected(ix == self.active_item_index())
+                .selected(is_active)
                 .on_click(cx.listener(move |pane: &mut Self, event, cx| {
                     pane.activate_item(ix, true, true, cx)
                 }))

crates/workspace2/src/shared_screen.rs 🔗

@@ -12,7 +12,7 @@ use gpui::{
     VisualContext, WindowContext,
 };
 use std::sync::{Arc, Weak};
-use ui::{h_stack, Icon, IconElement};
+use ui::{h_stack, prelude::*, Icon, IconElement, Label};
 
 pub enum Event {
     Close,
@@ -90,14 +90,22 @@ impl Item for SharedScreen {
         }
     }
 
-    fn tab_content(&self, _: Option<usize>, _: &WindowContext<'_>) -> gpui::AnyElement {
+    fn tab_content(
+        &self,
+        _: Option<usize>,
+        selected: bool,
+        _: &WindowContext<'_>,
+    ) -> gpui::AnyElement {
         h_stack()
             .gap_1()
             .child(IconElement::new(Icon::Screen))
-            .child(SharedString::from(format!(
-                "{}'s screen",
-                self.user.github_login
-            )))
+            .child(
+                Label::new(format!("{}'s screen", self.user.github_login)).color(if selected {
+                    Color::Default
+                } else {
+                    Color::Muted
+                }),
+            )
             .into_any()
     }