Skip to content

Commit

Permalink
Refactor Spacing into DynamicSpacing using proc macro (#20504)
Browse files Browse the repository at this point in the history
Density tracking issue: #18078 

This PR refactors our spacing system to use a more flexible and
maintainable approach. We've replaced the static `Spacing` enum with a
dynamically generated `DynamicSpacing` enum using a proc macro.

Enum variants now use a `BaseXX` format, where XX = the pixel value @
default rem size and the default UI density.

For example:

`CustomSpacing::Base16` would return 16px at the default UI scale &
density.

I'd love to find another name other than `Base` that is clear (to avoid
base_10, etc confusion), let me know if you have any ideas!

Changes:

- Introduced a new `derive_dynamic_spacing` proc macro to generate the
`DynamicSpacing` enum
- Updated all usages of `Spacing` to use the new `DynamicSpacing`
- Removed the `custom_spacing` function, mapping previous usages to
appropriate `DynamicSpacing` variants
- Improved documentation and type safety for spacing values

New usage example:

```rust
.child(
    div()
        .flex()
        .flex_none()
        .m(DynamicSpacing::Base04.px(cx))
        .size(DynamicSpacing::Base16.rems(cx))
        .children(icon),
)
```

vs old usage example:

```
.child(
    div()
        .flex()
        .flex_none()
        .m(Spacing::Small.px(cx))
        .size(custom_spacing(px(16.)))
        .children(icon),
)
```

Release Notes:

- N/A
  • Loading branch information
iamnbutler authored Nov 11, 2024
1 parent 93ab6ad commit 94d8ead
Show file tree
Hide file tree
Showing 29 changed files with 294 additions and 193 deletions.
8 changes: 4 additions & 4 deletions crates/assistant/src/assistant_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl AssistantPanel {
);
let _pane = cx.view().clone();
let right_children = h_flex()
.gap(Spacing::XSmall.rems(cx))
.gap(DynamicSpacing::Base02.rems(cx))
.child(
IconButton::new("new-chat", IconName::Plus)
.on_click(
Expand Down Expand Up @@ -4838,7 +4838,7 @@ impl ConfigurationView {
)
.child(
div()
.p(Spacing::Large.rems(cx))
.p(DynamicSpacing::Base08.rems(cx))
.bg(cx.theme().colors().surface_background)
.border_1()
.border_color(cx.theme().colors().border_variant)
Expand Down Expand Up @@ -4872,7 +4872,7 @@ impl Render for ConfigurationView {
.overflow_y_scroll()
.child(
v_flex()
.p(Spacing::XXLarge.rems(cx))
.p(DynamicSpacing::Base16.rems(cx))
.border_b_1()
.border_color(cx.theme().colors().border)
.gap_1()
Expand All @@ -4886,7 +4886,7 @@ impl Render for ConfigurationView {
)
.child(
v_flex()
.p(Spacing::XXLarge.rems(cx))
.p(DynamicSpacing::Base16.rems(cx))
.mt_1()
.gap_6()
.flex_1()
Expand Down
16 changes: 8 additions & 8 deletions crates/assistant/src/prompt_library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ impl PromptLibrary {
.overflow_x_hidden()
.child(
h_flex()
.p(Spacing::Small.rems(cx))
.p(DynamicSpacing::Base04.rems(cx))
.h_9()
.w_full()
.flex_none()
Expand Down Expand Up @@ -871,17 +871,17 @@ impl PromptLibrary {
.size_full()
.relative()
.overflow_hidden()
.pl(Spacing::XXLarge.rems(cx))
.pt(Spacing::Large.rems(cx))
.pl(DynamicSpacing::Base16.rems(cx))
.pt(DynamicSpacing::Base08.rems(cx))
.on_click(cx.listener(move |_, _, cx| {
cx.focus(&focus_handle);
}))
.child(
h_flex()
.group("active-editor-header")
.pr(Spacing::XXLarge.rems(cx))
.pt(Spacing::XSmall.rems(cx))
.pb(Spacing::Large.rems(cx))
.pr(DynamicSpacing::Base16.rems(cx))
.pt(DynamicSpacing::Base02.rems(cx))
.pb(DynamicSpacing::Base08.rems(cx))
.justify_between()
.child(
h_flex().gap_1().child(
Expand Down Expand Up @@ -943,13 +943,13 @@ impl PromptLibrary {
.child(
h_flex()
.h_full()
.gap(Spacing::XXLarge.rems(cx))
.gap(DynamicSpacing::Base16.rems(cx))
.child(div()),
)
.child(
h_flex()
.h_full()
.gap(Spacing::XXLarge.rems(cx))
.gap(DynamicSpacing::Base16.rems(cx))
.children(prompt_editor.token_count.map(
|token_count| {
let token_count: SharedString =
Expand Down
8 changes: 4 additions & 4 deletions crates/outline_panel/src/outline_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use serde::{Deserialize, Serialize};
use settings::{Settings, SettingsStore};
use smol::channel;
use theme::{SyntaxTheme, ThemeSettings};
use ui::{IndentGuideColors, IndentGuideLayout};
use ui::{DynamicSpacing, IndentGuideColors, IndentGuideLayout};
use util::{debug_panic, RangeExt, ResultExt, TryFutureExt};
use workspace::{
dock::{DockPosition, Panel, PanelEvent},
Expand All @@ -51,8 +51,8 @@ use workspace::{
ui::{
h_flex, v_flex, ActiveTheme, ButtonCommon, Clickable, Color, ContextMenu, FluentBuilder,
HighlightedLabel, Icon, IconButton, IconButtonShape, IconName, IconSize, Label,
LabelCommon, ListItem, Scrollbar, ScrollbarState, Selectable, Spacing, StyledExt,
StyledTypography, Tooltip,
LabelCommon, ListItem, Scrollbar, ScrollbarState, Selectable, StyledExt, StyledTypography,
Tooltip,
},
OpenInTerminal, WeakItemHandle, Workspace,
};
Expand Down Expand Up @@ -3867,7 +3867,7 @@ impl OutlinePanel {
})
.child(
h_flex()
.pt(Spacing::Small.rems(cx))
.pt(DynamicSpacing::Base04.rems(cx))
.justify_center()
.child({
let keystroke = match self.position(cx) {
Expand Down
2 changes: 1 addition & 1 deletion crates/quick_action_bar/src/quick_action_bar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ impl Render for QuickActionBar {

h_flex()
.id("quick action bar")
.gap(Spacing::Medium.rems(cx))
.gap(DynamicSpacing::Base06.rems(cx))
.children(self.render_repl_menu(cx))
.children(self.render_toggle_markdown_preview(self.workspace.clone(), cx))
.children(search_button)
Expand Down
6 changes: 3 additions & 3 deletions crates/recent_projects/src/ssh_connections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ impl RenderOnce for SshConnectionHeader {
};

h_flex()
.px(Spacing::XLarge.rems(cx))
.pt(Spacing::Large.rems(cx))
.pb(Spacing::Small.rems(cx))
.px(DynamicSpacing::Base12.rems(cx))
.pt(DynamicSpacing::Base08.rems(cx))
.pb(DynamicSpacing::Base04.rems(cx))
.rounded_t_md()
.w_full()
.gap_1p5()
Expand Down
10 changes: 5 additions & 5 deletions crates/repl/src/notebook/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ pub trait RenderableCell: Render {
if (cell_position == Some(&CellPosition::First) && is_first)
|| (cell_position == Some(&CellPosition::Last) && !is_first)
{
Some(div().flex().w_full().h(Spacing::XLarge.px(cx)))
Some(div().flex().w_full().h(DynamicSpacing::Base12.px(cx)))
} else {
None
}
Expand Down Expand Up @@ -389,7 +389,7 @@ impl Render for MarkdownCell {
.pr_6()
.rounded_sm()
.items_start()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.bg(self.selected_bg_color(cx))
.child(self.gutter(cx))
.child(
Expand Down Expand Up @@ -564,7 +564,7 @@ impl Render for CodeCell {
.pr_6()
.rounded_sm()
.items_start()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.bg(self.selected_bg_color(cx))
.child(self.gutter(cx))
.child(
Expand All @@ -590,7 +590,7 @@ impl Render for CodeCell {
.pr_6()
.rounded_sm()
.items_start()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.bg(self.selected_bg_color(cx))
.child(self.gutter_output(cx))
.child(
Expand Down Expand Up @@ -710,7 +710,7 @@ impl Render for RawCell {
.pr_2()
.rounded_sm()
.items_start()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.bg(self.selected_bg_color(cx))
.child(self.gutter(cx))
.child(
Expand Down
14 changes: 7 additions & 7 deletions crates/repl/src/notebook/notebook_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl NotebookEditor {

fn button_group(cx: &ViewContext<Self>) -> Div {
v_flex()
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.items_center()
.w(px(CONTROL_SIZE + 4.0))
.overflow_hidden()
Expand All @@ -299,14 +299,14 @@ impl NotebookEditor {
v_flex()
.max_w(px(CONTROL_SIZE + 4.0))
.items_center()
.gap(Spacing::XXLarge.rems(cx))
.gap(DynamicSpacing::Base16.rems(cx))
.justify_between()
.flex_none()
.h_full()
.py(Spacing::XLarge.px(cx))
.py(DynamicSpacing::Base12.px(cx))
.child(
v_flex()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.child(
Self::button_group(cx)
.child(
Expand Down Expand Up @@ -390,7 +390,7 @@ impl NotebookEditor {
)
.child(
v_flex()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.items_center()
.child(Self::render_notebook_control(
"more-menu",
Expand Down Expand Up @@ -468,8 +468,8 @@ impl Render for NotebookEditor {
.items_start()
.size_full()
.overflow_hidden()
.px(Spacing::XLarge.px(cx))
.gap(Spacing::XLarge.px(cx))
.px(DynamicSpacing::Base12.px(cx))
.gap(DynamicSpacing::Base12.px(cx))
.bg(cx.theme().colors().tab_bar_background)
.child(
v_flex()
Expand Down
8 changes: 5 additions & 3 deletions crates/ui/src/components/button/button.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#![allow(missing_docs)]
use gpui::{AnyView, DefiniteLength};

use crate::{prelude::*, ElevationIndex, IconPosition, KeyBinding, Spacing, TintColor};
use crate::{
prelude::*, Color, DynamicSpacing, ElevationIndex, IconPosition, KeyBinding, TintColor,
};
use crate::{
ButtonCommon, ButtonLike, ButtonSize, ButtonStyle, IconName, IconSize, Label, LineHeightStyle,
};
Expand Down Expand Up @@ -398,7 +400,7 @@ impl RenderOnce for Button {

self.base.child(
h_flex()
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.when(self.icon_position == Some(IconPosition::Start), |this| {
this.children(self.icon.map(|icon| {
ButtonIcon::new(icon)
Expand All @@ -412,7 +414,7 @@ impl RenderOnce for Button {
})
.child(
h_flex()
.gap(Spacing::Medium.rems(cx))
.gap(DynamicSpacing::Base06.rems(cx))
.justify_between()
.child(
Label::new(label)
Expand Down
10 changes: 6 additions & 4 deletions crates/ui/src/components/button/button_like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use gpui::{relative, CursorStyle, DefiniteLength, MouseButton};
use gpui::{transparent_black, AnyElement, AnyView, ClickEvent, Hsla, Rems};
use smallvec::SmallVec;

use crate::{prelude::*, ElevationIndex, Spacing};
use crate::{prelude::*, DynamicSpacing, ElevationIndex};

/// A trait for buttons that can be Selected. Enables setting the [`ButtonStyle`] of a button when it is selected.
pub trait SelectableButton: Selectable {
Expand Down Expand Up @@ -491,10 +491,12 @@ impl RenderOnce for ButtonLike {
ButtonLikeRounding::Left => this.rounded_l_md(),
ButtonLikeRounding::Right => this.rounded_r_md(),
})
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.map(|this| match self.size {
ButtonSize::Large => this.px(Spacing::Medium.rems(cx)),
ButtonSize::Default | ButtonSize::Compact => this.px(Spacing::Small.rems(cx)),
ButtonSize::Large => this.px(DynamicSpacing::Base06.rems(cx)),
ButtonSize::Default | ButtonSize::Compact => {
this.px(DynamicSpacing::Base04.rems(cx))
}
ButtonSize::None => this,
})
.bg(style.enabled(self.layer, cx).background)
Expand Down
8 changes: 4 additions & 4 deletions crates/ui/src/components/checkbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,16 @@ impl RenderOnce for Checkbox {
.id(self.id)
.justify_center()
.items_center()
.size(crate::styles::custom_spacing(cx, 20.))
.size(DynamicSpacing::Base20.rems(cx))
.group(group_id.clone())
.child(
div()
.flex()
.flex_none()
.justify_center()
.items_center()
.m(Spacing::Small.px(cx))
.size(crate::styles::custom_spacing(cx, 16.))
.m(DynamicSpacing::Base04.px(cx))
.size(DynamicSpacing::Base16.rems(cx))
.rounded_sm()
.bg(bg_color)
.border_1()
Expand Down Expand Up @@ -191,7 +191,7 @@ impl CheckboxWithLabel {
impl RenderOnce for CheckboxWithLabel {
fn render(self, cx: &mut WindowContext) -> impl IntoElement {
h_flex()
.gap(Spacing::Large.rems(cx))
.gap(DynamicSpacing::Base08.rems(cx))
.child(Checkbox::new(self.id.clone(), self.checked).on_click({
let on_click = self.on_click.clone();
move |checked, cx| {
Expand Down
8 changes: 4 additions & 4 deletions crates/ui/src/components/icon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ impl IconSize {
pub fn square_components(&self, cx: &mut WindowContext) -> (Pixels, Pixels) {
let icon_size = self.rems() * cx.rem_size();
let padding = match self {
IconSize::Indicator => Spacing::None.px(cx),
IconSize::XSmall => Spacing::XSmall.px(cx),
IconSize::Small => Spacing::XSmall.px(cx),
IconSize::Medium => Spacing::XSmall.px(cx),
IconSize::Indicator => DynamicSpacing::Base00.px(cx),
IconSize::XSmall => DynamicSpacing::Base02.px(cx),
IconSize::Small => DynamicSpacing::Base02.px(cx),
IconSize::Medium => DynamicSpacing::Base02.px(cx),
};

(icon_size, padding)
Expand Down
2 changes: 1 addition & 1 deletion crates/ui/src/components/keybinding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl RenderOnce for KeyBinding {
.join(" ")
)
})
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.flex_none()
.children(self.key_binding.keystrokes().iter().map(|keystroke| {
let key_icon = self.icon_for_key(keystroke);
Expand Down
2 changes: 1 addition & 1 deletion crates/ui/src/components/list/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl RenderOnce for List {
fn render(self, cx: &mut WindowContext) -> impl IntoElement {
v_flex()
.w_full()
.py(Spacing::Small.rems(cx))
.py(DynamicSpacing::Base04.rems(cx))
.children(self.header)
.map(|this| match (self.children.is_empty(), self.toggle) {
(false, _) => this.children(self.children),
Expand Down
6 changes: 3 additions & 3 deletions crates/ui/src/components/list/list_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,18 @@ impl RenderOnce for ListHeader {
.items_center()
.justify_between()
.w_full()
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.child(
h_flex()
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.children(self.toggle.map(|is_open| {
Disclosure::new("toggle", is_open).on_toggle(self.on_toggle.clone())
}))
.child(
div()
.id("label_container")
.flex()
.gap(Spacing::Small.rems(cx))
.gap(DynamicSpacing::Base04.rems(cx))
.items_center()
.children(self.start_slot)
.child(Label::new(self.label.clone()).color(Color::Muted))
Expand Down
Loading

0 comments on commit 94d8ead

Please sign in to comment.