editor: Make `TextAlign::Center` and `TextAlign::Right` work (#45417)

Danilo Leal created

Closes https://github.com/zed-industries/zed/issues/43208

This PR essentially unblocks the editable number field. The function
that shapes editor lines was hard-coding text alignment to the left,
meaning that whatever different alignment we'd pass through
`EditorStyles`would be ignored. To solve this, I just added a text align
and align width fields to the line paint function and updated all call
sites keeping the default configuration. Had to also add an
`alignment_offset()` helper to make sure the cursor positioning, the
selection background element, and the click-to-focus functionality were
kept in-sync with the non-left aligned editor.

Then... the big star of the show here is being able to add the `mode`
method to the number field, which uses `TextAlign::Center`, thus making
it work as we designed it to work.


https://github.com/user-attachments/assets/3539c976-d7bf-4d94-8188-a14328f94fbf

Next up, is turning the number filed to edit mode where applicable.

Release Notes:

- Fixed a bug where different text alignment configurations (i.e.,
center and right-aligned) wouldn't take effect in editors.

Change summary

crates/editor/src/element.rs                 | 106 +++++++++++++++++----
crates/gpui/examples/input.rs                |  11 +
crates/gpui/src/text_system/line.rs          |  12 +
crates/terminal_view/src/terminal_element.rs |  19 +++
crates/ui_input/src/number_field.rs          |  66 +++++++++----
5 files changed, 164 insertions(+), 50 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -46,9 +46,9 @@ use gpui::{
     KeybindingKeystroke, Length, Modifiers, ModifiersChangedEvent, MouseButton, MouseClickEvent,
     MouseDownEvent, MouseMoveEvent, MousePressureEvent, MouseUpEvent, PaintQuad, ParentElement,
     Pixels, PressureStage, ScrollDelta, ScrollHandle, ScrollWheelEvent, ShapedLine, SharedString,
-    Size, StatefulInteractiveElement, Style, Styled, TextRun, TextStyleRefinement, WeakEntity,
-    Window, anchored, deferred, div, fill, linear_color_stop, linear_gradient, outline, point, px,
-    quad, relative, size, solid_background, transparent_black,
+    Size, StatefulInteractiveElement, Style, Styled, TextAlign, TextRun, TextStyleRefinement,
+    WeakEntity, Window, anchored, deferred, div, fill, linear_color_stop, linear_gradient, outline,
+    point, px, quad, relative, size, solid_background, transparent_black,
 };
 use itertools::Itertools;
 use language::{IndentGuideSettings, language_settings::ShowWhitespaceSetting};
@@ -1695,9 +1695,13 @@ impl EditorElement {
                         [cursor_position.row().minus(visible_display_row_range.start) as usize];
                     let cursor_column = cursor_position.column() as usize;
 
-                    let cursor_character_x = cursor_row_layout.x_for_index(cursor_column);
-                    let mut block_width =
-                        cursor_row_layout.x_for_index(cursor_column + 1) - cursor_character_x;
+                    let cursor_character_x = cursor_row_layout.x_for_index(cursor_column)
+                        + cursor_row_layout
+                            .alignment_offset(self.style.text.text_align, text_hitbox.size.width);
+                    let cursor_next_x = cursor_row_layout.x_for_index(cursor_column + 1)
+                        + cursor_row_layout
+                            .alignment_offset(self.style.text.text_align, text_hitbox.size.width);
+                    let mut block_width = cursor_next_x - cursor_character_x;
                     if block_width == Pixels::ZERO {
                         block_width = em_advance;
                     }
@@ -6160,10 +6164,25 @@ impl EditorElement {
                     let color = cx.theme().colors().editor_hover_line_number;
 
                     let line = self.shape_line_number(shaped_line.text.clone(), color, window);
-                    line.paint(hitbox.origin, line_height, window, cx).log_err()
+                    line.paint(
+                        hitbox.origin,
+                        line_height,
+                        TextAlign::Left,
+                        None,
+                        window,
+                        cx,
+                    )
+                    .log_err()
                 } else {
                     shaped_line
-                        .paint(hitbox.origin, line_height, window, cx)
+                        .paint(
+                            hitbox.origin,
+                            line_height,
+                            TextAlign::Left,
+                            None,
+                            window,
+                            cx,
+                        )
                         .log_err()
                 }) else {
                     continue;
@@ -7252,23 +7271,27 @@ impl EditorElement {
                     .map(|row| {
                         let line_layout =
                             &layout.position_map.line_layouts[row.minus(start_row) as usize];
+                        let alignment_offset =
+                            line_layout.alignment_offset(layout.text_align, layout.content_width);
                         HighlightedRangeLine {
                             start_x: if row == range.start.row() {
                                 layout.content_origin.x
                                     + Pixels::from(
                                         ScrollPixelOffset::from(
-                                            line_layout.x_for_index(range.start.column() as usize),
+                                            line_layout.x_for_index(range.start.column() as usize)
+                                                + alignment_offset,
                                         ) - layout.position_map.scroll_pixel_position.x,
                                     )
                             } else {
-                                layout.content_origin.x
+                                layout.content_origin.x + alignment_offset
                                     - Pixels::from(layout.position_map.scroll_pixel_position.x)
                             },
                             end_x: if row == range.end.row() {
                                 layout.content_origin.x
                                     + Pixels::from(
                                         ScrollPixelOffset::from(
-                                            line_layout.x_for_index(range.end.column() as usize),
+                                            line_layout.x_for_index(range.end.column() as usize)
+                                                + alignment_offset,
                                         ) - layout.position_map.scroll_pixel_position.x,
                                     )
                             } else {
@@ -7276,6 +7299,7 @@ impl EditorElement {
                                     ScrollPixelOffset::from(
                                         layout.content_origin.x
                                             + line_layout.width
+                                            + alignment_offset
                                             + line_end_overshoot,
                                     ) - layout.position_map.scroll_pixel_position.x,
                                 )
@@ -8516,8 +8540,15 @@ impl LineWithInvisibles {
         for fragment in &self.fragments {
             match fragment {
                 LineFragment::Text(line) => {
-                    line.paint(fragment_origin, line_height, window, cx)
-                        .log_err();
+                    line.paint(
+                        fragment_origin,
+                        line_height,
+                        layout.text_align,
+                        Some(layout.content_width),
+                        window,
+                        cx,
+                    )
+                    .log_err();
                     fragment_origin.x += line.width;
                 }
                 LineFragment::Element { size, .. } => {
@@ -8559,8 +8590,15 @@ impl LineWithInvisibles {
         for fragment in &self.fragments {
             match fragment {
                 LineFragment::Text(line) => {
-                    line.paint_background(fragment_origin, line_height, window, cx)
-                        .log_err();
+                    line.paint_background(
+                        fragment_origin,
+                        line_height,
+                        layout.text_align,
+                        Some(layout.content_width),
+                        window,
+                        cx,
+                    )
+                    .log_err();
                     fragment_origin.x += line.width;
                 }
                 LineFragment::Element { size, .. } => {
@@ -8609,7 +8647,7 @@ impl LineWithInvisibles {
                 [token_offset, token_end_offset],
                 Box::new(move |window: &mut Window, cx: &mut App| {
                     invisible_symbol
-                        .paint(origin, line_height, window, cx)
+                        .paint(origin, line_height, TextAlign::Left, None, window, cx)
                         .log_err();
                 }),
             )
@@ -8770,6 +8808,15 @@ impl LineWithInvisibles {
 
         None
     }
+
+    pub fn alignment_offset(&self, text_align: TextAlign, content_width: Pixels) -> Pixels {
+        let line_width = self.width;
+        match text_align {
+            TextAlign::Left => px(0.0),
+            TextAlign::Center => (content_width - line_width) / 2.0,
+            TextAlign::Right => content_width - line_width,
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -10172,6 +10219,8 @@ impl Element for EditorElement {
                         em_width,
                         em_advance,
                         snapshot,
+                        text_align: self.style.text.text_align,
+                        content_width: text_hitbox.size.width,
                         gutter_hitbox: gutter_hitbox.clone(),
                         text_hitbox: text_hitbox.clone(),
                         inline_blame_bounds: inline_blame_layout
@@ -10225,6 +10274,8 @@ impl Element for EditorElement {
                         sticky_buffer_header,
                         sticky_headers,
                         expand_toggles,
+                        text_align: self.style.text.text_align,
+                        content_width: text_hitbox.size.width,
                     }
                 })
             })
@@ -10405,6 +10456,8 @@ pub struct EditorLayout {
     sticky_buffer_header: Option<AnyElement>,
     sticky_headers: Option<StickyHeaders>,
     document_colors: Option<(DocumentColorsRenderMode, Vec<(Range<DisplayPoint>, Hsla)>)>,
+    text_align: TextAlign,
+    content_width: Pixels,
 }
 
 struct StickyHeaders {
@@ -10572,7 +10625,9 @@ impl StickyHeaderLine {
                 gutter_origin.x + gutter_width - gutter_right_padding - line_number.width,
                 gutter_origin.y,
             );
-            line_number.paint(origin, line_height, window, cx).log_err();
+            line_number
+                .paint(origin, line_height, TextAlign::Left, None, window, cx)
+                .log_err();
         }
     }
 }
@@ -11011,6 +11066,8 @@ pub(crate) struct PositionMap {
     pub visible_row_range: Range<DisplayRow>,
     pub line_layouts: Vec<LineWithInvisibles>,
     pub snapshot: EditorSnapshot,
+    pub text_align: TextAlign,
+    pub content_width: Pixels,
     pub text_hitbox: Hitbox,
     pub gutter_hitbox: Hitbox,
     pub inline_blame_bounds: Option<(Bounds<Pixels>, BufferId, BlameEntry)>,
@@ -11076,10 +11133,12 @@ impl PositionMap {
             .line_layouts
             .get(row as usize - scroll_position.y as usize)
         {
-            if let Some(ix) = line.index_for_x(x) {
+            let alignment_offset = line.alignment_offset(self.text_align, self.content_width);
+            let x_relative_to_text = x - alignment_offset;
+            if let Some(ix) = line.index_for_x(x_relative_to_text) {
                 (ix as u32, px(0.))
             } else {
-                (line.len as u32, px(0.).max(x - line.width))
+                (line.len as u32, px(0.).max(x_relative_to_text - line.width))
             }
         } else {
             (0, x)
@@ -11268,7 +11327,14 @@ impl CursorLayout {
 
         if let Some(block_text) = &self.block_text {
             block_text
-                .paint(self.origin + origin, self.line_height, window, cx)
+                .paint(
+                    self.origin + origin,
+                    self.line_height,
+                    TextAlign::Left,
+                    None,
+                    window,
+                    cx,
+                )
                 .log_err();
         }
     }

crates/gpui/examples/input.rs 🔗

@@ -546,8 +546,15 @@ impl Element for TextElement {
             window.paint_quad(selection)
         }
         let line = prepaint.line.take().unwrap();
-        line.paint(bounds.origin, window.line_height(), window, cx)
-            .unwrap();
+        line.paint(
+            bounds.origin,
+            window.line_height(),
+            gpui::TextAlign::Left,
+            None,
+            window,
+            cx,
+        )
+        .unwrap();
 
         if focus_handle.is_focused(window)
             && let Some(cursor) = prepaint.cursor.take()

crates/gpui/src/text_system/line.rs 🔗

@@ -64,6 +64,8 @@ impl ShapedLine {
         &self,
         origin: Point<Pixels>,
         line_height: Pixels,
+        align: TextAlign,
+        align_width: Option<Pixels>,
         window: &mut Window,
         cx: &mut App,
     ) -> Result<()> {
@@ -71,8 +73,8 @@ impl ShapedLine {
             origin,
             &self.layout,
             line_height,
-            TextAlign::default(),
-            None,
+            align,
+            align_width,
             &self.decoration_runs,
             &[],
             window,
@@ -87,6 +89,8 @@ impl ShapedLine {
         &self,
         origin: Point<Pixels>,
         line_height: Pixels,
+        align: TextAlign,
+        align_width: Option<Pixels>,
         window: &mut Window,
         cx: &mut App,
     ) -> Result<()> {
@@ -94,8 +98,8 @@ impl ShapedLine {
             origin,
             &self.layout,
             line_height,
-            TextAlign::default(),
-            None,
+            align,
+            align_width,
             &self.decoration_runs,
             &[],
             window,

crates/terminal_view/src/terminal_element.rs 🔗

@@ -151,7 +151,14 @@ impl BatchedTextRun {
                 std::slice::from_ref(&self.style),
                 Some(dimensions.cell_width),
             )
-            .paint(pos, dimensions.line_height, window, cx);
+            .paint(
+                pos,
+                dimensions.line_height,
+                gpui::TextAlign::Left,
+                None,
+                window,
+                cx,
+            );
     }
 }
 
@@ -1326,8 +1333,14 @@ impl Element for TerminalElement {
                                     }],
                                     None
                                 );
-                                shaped_line
-                                    .paint(ime_position, layout.dimensions.line_height, window, cx)
+                                shaped_line.paint(
+                                    ime_position,
+                                    layout.dimensions.line_height,
+                                    gpui::TextAlign::Left,
+                                    None,
+                                    window,
+                                    cx,
+                                )
                                     .log_err();
                             }
 

crates/ui_input/src/number_field.rs 🔗

@@ -5,8 +5,11 @@ use std::{
     str::FromStr,
 };
 
-use editor::{Editor, EditorStyle};
-use gpui::{ClickEvent, Entity, FocusHandle, Focusable, FontWeight, Modifiers};
+use editor::Editor;
+use gpui::{
+    ClickEvent, Entity, FocusHandle, Focusable, FontWeight, Modifiers, TextAlign,
+    TextStyleRefinement,
+};
 
 use settings::{CenteredPaddingSettings, CodeFade, DelayMs, InactiveOpacity, MinimumContrast};
 use ui::prelude::*;
@@ -309,6 +312,11 @@ impl<T: NumberFieldType> NumberField<T> {
         self
     }
 
+    pub fn mode(self, mode: NumberFieldMode, cx: &mut App) -> Self {
+        self.mode.write(cx, mode);
+        self
+    }
+
     pub fn on_reset(
         mut self,
         on_reset: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static,
@@ -451,9 +459,11 @@ impl<T: NumberFieldType> RenderOnce for NumberField<T> {
                                         |window, cx| {
                                             let previous_focus_handle = window.focused(cx);
                                             let mut editor = Editor::single_line(window, cx);
-                                            let mut style = EditorStyle::default();
-                                            style.text.text_align = gpui::TextAlign::Right;
-                                            editor.set_style(style, window, cx);
+
+                                            editor.set_text_style_refinement(TextStyleRefinement {
+                                                text_align: Some(TextAlign::Center),
+                                                ..Default::default()
+                                            });
 
                                             editor.set_text(format!("{}", self.value), window, cx);
                                             cx.on_focus_out(&editor.focus_handle(cx), window, {
@@ -555,22 +565,36 @@ impl Component for NumberField<usize> {
         Some(
             v_flex()
                 .gap_6()
-                .children(vec![single_example(
-                    "Default Numeric Stepper",
-                    NumberField::new(
-                        "numeric-stepper-component-preview",
-                        *stepper_example.read(cx),
-                        window,
-                        cx,
-                    )
-                    .on_change({
-                        let stepper_example = stepper_example.clone();
-                        move |value, _, cx| stepper_example.write(cx, *value)
-                    })
-                    .min(1.0)
-                    .max(100.0)
-                    .into_any_element(),
-                )])
+                .children(vec![
+                    single_example(
+                        "Default Number Field",
+                        NumberField::new("number-field", *stepper_example.read(cx), window, cx)
+                            .on_change({
+                                let stepper_example = stepper_example.clone();
+                                move |value, _, cx| stepper_example.write(cx, *value)
+                            })
+                            .min(1.0)
+                            .max(100.0)
+                            .into_any_element(),
+                    ),
+                    single_example(
+                        "Read-Only Number Field",
+                        NumberField::new(
+                            "editable-number-field",
+                            *stepper_example.read(cx),
+                            window,
+                            cx,
+                        )
+                        .on_change({
+                            let stepper_example = stepper_example.clone();
+                            move |value, _, cx| stepper_example.write(cx, *value)
+                        })
+                        .min(1.0)
+                        .max(100.0)
+                        .mode(NumberFieldMode::Edit, cx)
+                        .into_any_element(),
+                    ),
+                ])
                 .into_any_element(),
         )
     }