Skip to content

fix(connections): quieter core rows and a Save that knows if there is anything to save #443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kirillDevPro merged 1 commit into from
Sep 6, 2026
Merged

fix(connections): quieter core rows and a Save that knows if there is anything to save #443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions crates/moon-ui-gpui/src/settings/connections/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use std::sync::atomic::{AtomicU64, Ordering};

use gpui::*;
use moon_ui::{MoonColorPickerState, MoonInputEvent, MoonInputState};
use rust_i18n::t;

use super::SettingsView;
use crate::Backend;
Expand Down Expand Up @@ -60,7 +61,7 @@ pub(super) struct ConnRow {
color: Entity<MoonColorPickerState>,
}

/// The ten per-row element-id strings the row factory used to rebuild with `format!` on every
/// The thirteen per-row element-id strings the row factory used to rebuild with `format!` on every
/// frame. Built once in [`build_conn`] and read from thereafter, so `server_row` allocates none of
/// them.
///
Expand All @@ -73,7 +74,15 @@ pub(super) struct ConnRowIds {
pub(super) group: SharedString,
pub(super) bundle: SharedString,
pub(super) feed: SharedString,
/// Id of the interactive wrapper that carries the data cell's tooltip.
///
/// Separate from [`Self::feed`]: `MoonDropdown` has no tooltip prop, so the tooltip lives on
/// a `div` around it, and gpui needs its own id for an interactive element.
pub(super) feed_tip: SharedString,
pub(super) proto: SharedString,
/// Id of the interactive wrapper that carries the proto cell's tooltip. See
/// [`Self::feed_tip`].
pub(super) proto_tip: SharedString,
pub(super) preset: SharedString,
pub(super) act: SharedString,
pub(super) win: SharedString,
Expand Down Expand Up @@ -117,7 +126,9 @@ impl ConnRowIds {
group: SharedString::from(format!("group-{ident}")),
bundle: SharedString::from(format!("bundle-{ident}")),
feed: SharedString::from(format!("feed-{ident}")),
feed_tip: SharedString::from(format!("feed-tip-{ident}")),
proto: SharedString::from(format!("proto-{ident}")),
proto_tip: SharedString::from(format!("proto-tip-{ident}")),
preset: SharedString::from(format!("preset-{ident}")),
act: SharedString::from(format!("act-{ident}")),
win: SharedString::from(format!("win-{ident}")),
Expand Down Expand Up @@ -151,6 +162,8 @@ pub(super) fn sync_groups_from_servers(
/// i: Draft index of the server field.
/// row_key: Per-session identity of the owning row.
/// init: Initial field value.
/// placeholder: Hint shown while the field is empty, or `None` for a field whose empty
/// state needs no explanation.
/// get: Accessor for the draft field.
/// set: Mutator for the draft field.
/// sync_groups: Whether a change must synchronize draft group rows.
Expand All @@ -163,11 +176,22 @@ fn conn_input(
i: usize,
row_key: u64,
init: String,
placeholder: Option<String>,
get: fn(&ServerConfig) -> String,
set: fn(&mut ServerConfig, String),
sync_groups: bool,
) -> Entity<MoonInputState> {
let st = cx.new(|cx| MoonInputState::new(window, cx).default_value(init));
// The placeholder belongs on the STATE, never on the `MoonInput` builder: MoonUI applies
// `MoonInput::placeholder` only inside its `self.state.unwrap_or_else(..)` branch, so a
// widget handed an external `.state(..)` -- which every field here is -- drops it silently.
// That is why the "Charts" column rendered as an empty box with no hint at all.
let st = cx.new(|cx| {
let st = MoonInputState::new(window, cx).default_value(init);
match placeholder {
Some(ph) => st.placeholder(ph),
None => st,
}
});
cx.subscribe(&st, move |this, emitter, ev: &MoonInputEvent, cx| {
if matches!(ev, MoonInputEvent::Change) {
let val = emitter.read(cx).value().to_string();
Expand Down Expand Up @@ -251,6 +275,9 @@ pub(super) fn build_conn(
i,
row_key,
s.name.clone(),
// No placeholder: a nameless core is not a state worth explaining, and the
// user's own text is the only thing this field ever holds.
None,
|s| s.name.clone(),
|s, v| s.name = v,
false,
Expand All @@ -264,6 +291,7 @@ pub(super) fn build_conn(
i,
row_key,
s.key.expose().to_string(),
Some(t!("conn.key_ph").to_string()),
|s| s.key.expose().to_string(),
|s, v| {
// Typing or Ctrl+V into the field fills a row's transport mode the
Expand All @@ -290,6 +318,9 @@ pub(super) fn build_conn(
i,
row_key,
s.group.clone(),
// No placeholder: `build_conn` is only ever handed saved or pending rows
// whose group defaults to "default", so the field is never empty in practice.
None,
|s| s.group.clone(),
|s, v| s.group = v,
true,
Expand All @@ -300,6 +331,10 @@ pub(super) fn build_conn(
i,
row_key,
s.chart_bundle.clone(),
// An empty bundle field is the DEFAULT, not an omission: the core then
// follows the global chart setting. The hint is what says so, and without it
// the column reads as a blank box nobody can interpret.
Some(t!("conn.bundle_ph").to_string()),
|s| s.chart_bundle.clone(),
|s, v| s.chart_bundle = v,
false,
Expand Down
125 changes: 94 additions & 31 deletions crates/moon-ui-gpui/src/settings/connections/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,35 @@ impl SettingsView {
}
}

/// Wrap one control in an interactive div carrying a wrapping tooltip.
///
/// Used where the control itself has no tooltip prop -- `MoonDropdown` has none -- and where a
/// cryptic label (`V0`, `8/8`) would otherwise be decodable only by finding its column heading.
/// gpui needs an id on an interactive element, so the caller supplies one from [`ConnRowIds`].
///
/// Args:
/// id: Element id of the wrapper, distinct from the control's own.
/// tip: Already-localized tooltip text.
/// max_w: Wrap width; the transport explanation is long enough to need more than the default.
/// control: The control to wrap.
///
/// Returns:
/// The control under a hover tooltip, occupying the same cell.
fn with_tip(
id: SharedString,
tip: SharedString,
max_w: f32,
control: impl IntoElement,
) -> impl IntoElement {
div()
.id(id)
.tooltip(move |_window, cx| {
cx.new(|_| MoonTooltipView::new(tip.clone()).max_width(max_w))
.into()
})
.child(control)
}

/// Build the `Data n/8` dropdown ported from egui's `feed_button`.
///
/// The trigger reports enabled feed flags; its eight checkbox items update the draft.
Expand All @@ -537,7 +566,7 @@ impl SettingsView {
/// cx: Application context.
///
/// Returns:
/// The feed-flag dropdown for one core row.
/// The feed-flag dropdown wrapped in its cell tooltip.
fn feed_popover(
view: &SettingsView,
weak: &WeakEntity<SettingsView>,
Expand Down Expand Up @@ -594,7 +623,9 @@ fn feed_popover(
// that MoonUI stores for the life of the element, and a strong handle there would close
// SettingsView -> element -> closure -> SettingsView and keep the window alive forever.
let view_weak = weak.clone();
MoonDropdown::new(ids.feed.clone())
// `8/8` is a count with no visible denominator meaning: the column tooltip already explains
// what the eight categories are and what the amber tint means, so reuse it on the cell.
let dropdown = MoonDropdown::new(ids.feed.clone())
.label(format!("{on}/8"))
.trigger_caret(true)
.trigger_variant(if tinted {
Expand All @@ -617,7 +648,13 @@ fn feed_popover(
this.feed_open = now_open.then_some(row_key);
cx.notify();
});
})
});
with_tip(
ids.feed_tip.clone(),
t!("conn.tip.flags").to_string().into(),
320.0,
dropdown,
)
}

/// Build the MoonProto transport selector for one server row.
Expand All @@ -635,11 +672,12 @@ fn feed_popover(
/// view: Settings state read for the row's current draft value.
/// weak: Weak owner the select handler closes over.
/// i: Draft index of the server being edited.
/// row_key: Owning row's identity, the value `proto_open` is compared against.
/// ids: Precomputed element ids for the row.
/// cx: Application context.
///
/// Returns:
/// A compact dropdown bound to draft `servers[i].transport`.
/// The transport dropdown, wrapped in its row-level explanatory tooltip.
fn proto_dropdown(
view: &SettingsView,
weak: &WeakEntity<SettingsView>,
Expand Down Expand Up @@ -704,7 +742,9 @@ fn proto_dropdown(
};

let view_weak = weak.clone();
MoonDropdown::new(ids.proto.clone())
// `V0` alone says nothing: the existing column tooltip is the only thing that explains the
// MoonProto trio, and a reader looking at a row should not have to find the heading first.
let dropdown = MoonDropdown::new(ids.proto.clone())
.label(cur.map_or(SharedString::from("-"), |v| SharedString::from(v.label())))
.trigger_caret(true)
.trigger_variant(MoonButtonVariant::Neutral)
Expand All @@ -720,7 +760,13 @@ fn proto_dropdown(
this.proto_open = now_open.then_some(row_key);
cx.notify();
});
})
});
with_tip(
ids.proto_tip.clone(),
t!("conn.tip.proto").to_string().into(),
320.0,
dropdown,
)
}

/// Build the workspace-preset selector for one server row.
Expand Down Expand Up @@ -946,8 +992,6 @@ pub(super) fn server_row(
MoonInput::new(ids.key.clone())
.state(&row.key)
.small()
// Indicate that this field expects a core key.
.placeholder(t!("conn.key_ph").to_string())
.mask_toggle()
// Allow the key to be cleared quickly before replacement.
.cleanable(true),
Expand All @@ -967,13 +1011,12 @@ pub(super) fn server_row(
.state(&row.group)
.small()
.into_any_element(),
// An empty bundle field is the DEFAULT, not an omission, so the placeholder names what
// the field would hold rather than nudging: it is a bundle NAME (`ChartBucket::Bundle`
// in `moon-core/src/config/servers.rs`), which an empty white cell said nothing about.
// Both placeholders here live on the STATE instead, in `super::conn_input`: MoonUI honours
// `MoonInput::placeholder` only for a widget that creates its own state, so setting it
// beside `.state(..)` drops it silently -- which is why this column drew as a blank box.
MoonInput::new(ids.bundle.clone())
.state(&row.bundle)
.small()
.placeholder(t!("conn.bundle_ph").to_string())
.into_any_element(),
feed_popover(view, weak, i, row_key, ids, cx).into_any_element(),
MoonColorPicker::new(&row.color).into_any_element(),
Expand Down Expand Up @@ -1055,10 +1098,20 @@ impl SettingsView {

/// Build one column heading, with its tooltip, ported from egui's `head_tip`.
///
/// Underlining and brighter text signal hover help. A column with no `label` -- colour,
/// delete, reconnect, status -- yields the bare cell, which still has to be emitted: the
/// header's growing columns only receive the same free space the rows give them when the
/// trailing widths are reserved too.
/// The heading reads like every other table header in the app: muted, unadorned, its tooltip
/// found by hovering rather than advertised. It used to be underlined in `text_soft` and set
/// in full-strength `text`, which made a sort-and-tooltip heading look like a hyperlink --
/// the one place in this codebase that did (`grep -n "underline()"`). The reference is
/// MoonUI's own `MoonDataTable` header (`moon/data_table/header.rs`), which paints
/// `style.header_text` -- `p.text_muted` -- and attaches its tooltip with no visual
/// affordance at all; Report and Orders are drawn by it.
///
/// Losing the affordance costs nothing here, because the two cryptic cells that actually
/// needed decoding -- `V0` and `8/8` -- now carry their own tooltips on the row itself.
///
/// A column with no `label` -- colour, delete, reconnect, status -- yields the bare cell,
/// which still has to be emitted: the header's growing columns only receive the same free
/// space the rows give them when the trailing widths are reserved too.
///
/// Args:
/// col: Which column to head.
Expand All @@ -1080,6 +1133,9 @@ impl SettingsView {
return base.into_any_element();
};
let tip: SharedString = t!(spec.tip.unwrap_or(label_key)).to_string().into();
// The id and the tooltip go on `base` ITSELF, never on a wrapper: `base` carries the
// column's `flex_basis`, cap and grow policy, so anything wrapped around it would become
// the flex item instead and the heading would drift off its own column.
base.id(spec.id)
.child(
div()
Expand All @@ -1089,9 +1145,7 @@ impl SettingsView {
// widen it, or the heading would push the grid it is describing.
.truncate()
.text_size(design::t_body(cx))
.text_color(rgb(p.text))
.underline()
.text_decoration_color(rgb(p.text_soft))
.text_color(rgb(p.text_muted))
.child(t!(label_key).to_string()),
)
.tooltip(move |_window, cx| {
Expand All @@ -1101,26 +1155,35 @@ impl SettingsView {
.into_any_element()
}

/// Build an arbitrary-width help label with underlining and a wrapping tooltip.
/// Build an arbitrary-width help label with a wrapping tooltip.
///
/// Used for section or group headings that need an explanation on hover rather than a column.
/// Bold `text` at full strength, matching `settings/common.rs:section`, and NOT underlined:
/// a heading is not a link. Same reasoning as [`Self::col_head`] above.
///
/// Args:
/// id: Stable identity for the tooltip wrapper.
/// label: Visible heading text.
/// tip: Already-localized explanatory tooltip text.
/// p: Active palette supplying the heading colour.
///
/// Returns:
/// The full-strength heading label wrapped in its explanatory tooltip.
pub(super) fn hint_label(
id: &'static str,
label: impl Into<SharedString>,
tip: SharedString,
p: MoonPalette,
) -> impl IntoElement {
div()
.id(id)
.font_bold()
.text_color(rgb(p.text))
.underline()
.text_decoration_color(rgb(p.text_soft))
.child(label.into())
.tooltip(move |_window, cx| {
cx.new(|_| MoonTooltipView::new(tip.clone()).max_width(360.0))
.into()
})
with_tip(
SharedString::from(id),
tip,
360.0,
div()
.font_bold()
.text_color(rgb(p.text))
.child(label.into()),
)
}

/// Render the core table header over the columns it names.
Expand Down
Loading