fix: Use the real text height for layout (#932)

* fix: Use the real text height for layout * clean up * chore: Clean up * fixes * fix tests
marc2332 · Sep 28, 2024 · 2531a94 · 2531a94
1 parent 198b95a
commit 2531a94
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 62 deletions.
diff --git a/crates/components/src/button.rs b/crates/components/src/button.rs
@@ -190,7 +190,6 @@ pub fn Button(
             text_align: "center",
             main_align: "center",
             cross_align: "center",
-            line_height: "1.1",
             {&children}
         }
     )

diff --git a/crates/components/src/tooltip.rs b/crates/components/src/tooltip.rs
@@ -84,11 +84,11 @@ pub fn TooltipContainer(
             onmouseenter,
             onmouseleave,
             {children},
-            if *is_hovering.read() {
-                rect {
-                    height: "0",
-                    width: "0",
-                    layer: "-1500",
+            rect {
+                height: "0",
+                width: "0",
+                layer: "-1500",
+                if *is_hovering.read() {
                     match position {
                         TooltipPosition::Below => rsx!(
                             rect {
@@ -107,7 +107,6 @@ pub fn TooltipContainer(
                             }
                         ),
                     }
-
                 }
             }
         }

diff --git a/crates/core/src/elements/paragraph.rs b/crates/core/src/elements/paragraph.rs
@@ -138,7 +138,7 @@ impl ElementUtils for ParagraphElement {
         };
 
         if node_cursor_state.position.is_some() {
-            let (paragraph, _) = create_paragraph(
+            let paragraph = create_paragraph(
                 node_ref,
                 &area.size,
                 font_collection,

diff --git a/crates/core/src/node.rs b/crates/core/src/node.rs
@@ -125,7 +125,7 @@ impl NodeState {
             ),
             (
                 "line_height",
-                AttributeType::Measure(self.font_style.line_height),
+                AttributeType::OptionalMeasure(self.font_style.line_height),
             ),
             (
                 "text_align",
@@ -160,6 +160,7 @@ pub enum AttributeType<'a> {
     Gradient(Fill),
     Size(&'a Size),
     Measure(f32),
+    OptionalMeasure(Option<f32>),
     Measures(Gaps),
     CornerRadius(CornerRadius),
     Direction(&'a DirectionMode),

diff --git a/crates/core/src/render/skia_measurer.rs b/crates/core/src/render/skia_measurer.rs
@@ -68,31 +68,32 @@ impl<'a> LayoutMeasurer<NodeId> for SkiaMeasurer<'a> {
 
         match &*node_type {
             NodeType::Element(ElementNode { tag, .. }) if tag == &TagName::Label => {
-                let (label, paragraph_font_height) = create_label(
+                let label = create_label(
                     &node,
                     area_size,
                     self.font_collection,
                     self.default_fonts,
                     self.scale_factor,
                 );
-
-                let res = Size2D::new(label.longest_line(), label.height());
+                let height = label.height();
+                let res = Size2D::new(label.longest_line(), height);
                 let mut map = SendAnyMap::new();
-                map.insert(CachedParagraph(label, paragraph_font_height));
+                map.insert(CachedParagraph(label, height));
                 Some((res, Arc::new(map)))
             }
             NodeType::Element(ElementNode { tag, .. }) if tag == &TagName::Paragraph => {
-                let (paragraph, paragraph_font_height) = create_paragraph(
+                let paragraph = create_paragraph(
                     &node,
                     area_size,
                     self.font_collection,
                     false,
                     self.default_fonts,
                     self.scale_factor,
                 );
-                let res = Size2D::new(paragraph.longest_line(), paragraph.height());
+                let height = paragraph.height();
+                let res = Size2D::new(paragraph.longest_line(), height);
                 let mut map = SendAnyMap::new();
-                map.insert(CachedParagraph(paragraph, paragraph_font_height));
+                map.insert(CachedParagraph(paragraph, height));
                 Some((res, Arc::new(map)))
             }
             _ => None,
@@ -130,7 +131,7 @@ pub fn create_label(
     font_collection: &FontCollection,
     default_font_family: &[String],
     scale_factor: f32,
-) -> (Paragraph, f32) {
+) -> Paragraph {
     let font_style = &*node.get::<FontStyleState>().unwrap();
 
     let mut paragraph_style = ParagraphStyle::default();
@@ -142,7 +143,7 @@ pub fn create_label(
         paragraph_style.set_ellipsis(ellipsis);
     }
 
-    let text_style = font_style.text_style(default_font_family, scale_factor, true);
+    let text_style = font_style.text_style(default_font_family, scale_factor);
     paragraph_style.set_text_style(&text_style);
 
     let mut paragraph_builder = ParagraphBuilder::new(&paragraph_style, font_collection);
@@ -160,16 +161,7 @@ pub fn create_label(
         area_size.width + 1.0
     });
 
-    // Measure the actual text height, ignoring the line height
-    let mut height = paragraph.height();
-    for line in paragraph.get_line_metrics() {
-        for (_, text) in line.get_style_metrics(0..1) {
-            let text_height = -(text.font_metrics.ascent - text.font_metrics.descent);
-            height = height.max(text_height);
-        }
-    }
-
-    (paragraph, height)
+    paragraph
 }
 
 /// Align the Y axis of the highlights and cursor of a paragraph
@@ -227,7 +219,7 @@ pub fn create_paragraph(
     is_rendering: bool,
     default_font_family: &[String],
     scale_factor: f32,
-) -> (Paragraph, f32) {
+) -> Paragraph {
     let font_style = &*node.get::<FontStyleState>().unwrap();
 
     let mut paragraph_style = ParagraphStyle::default();
@@ -241,13 +233,10 @@ pub fn create_paragraph(
 
     let mut paragraph_builder = ParagraphBuilder::new(&paragraph_style, font_collection);
 
-    let text_style = font_style.text_style(default_font_family, scale_factor, true);
+    let text_style = font_style.text_style(default_font_family, scale_factor);
     paragraph_builder.push_style(&text_style);
 
-    let node_children = node.children();
-    let node_children_len = node_children.len();
-
-    for text_span in node_children {
+    for text_span in node.children() {
         if let NodeType::Element(ElementNode {
             tag: TagName::Text, ..
         }) = &*text_span.node_type()
@@ -256,7 +245,7 @@ pub fn create_paragraph(
             let text_node = *text_nodes.first().unwrap();
             let text_node_type = &*text_node.node_type();
             let font_style = text_span.get::<FontStyleState>().unwrap();
-            let text_style = font_style.text_style(default_font_family, scale_factor, true);
+            let text_style = font_style.text_style(default_font_family, scale_factor);
             paragraph_builder.push_style(&text_style);
 
             if let NodeType::Text(text) = text_node_type {
@@ -277,14 +266,5 @@ pub fn create_paragraph(
         area_size.width + 1.0
     });
 
-    // Measure the actual text height, ignoring the line height
-    let mut height = paragraph.height();
-    for line in paragraph.get_line_metrics() {
-        for (_, text) in line.get_style_metrics(0..node_children_len) {
-            let text_height = -(text.font_metrics.ascent - text.font_metrics.descent);
-            height = height.max(text_height);
-        }
-    }
-
-    (paragraph, height)
+    paragraph
 }
diff --git a/crates/devtools/src/tabs/style.rs b/crates/devtools/src/tabs/style.rs
@@ -38,6 +38,15 @@ pub fn NodeInspectorStyle(node_id: String) -> Element {
                             }
                         }
                     }
+                    AttributeType::OptionalMeasure(measure) => {
+                        rsx!{
+                            Property {
+                                key: "{i}",
+                                name: "{name}",
+                                value: measure.map(|measure| measure.to_string()).unwrap_or_else(|| "inherit".to_string())
+                            }
+                        }
+                    }
                     AttributeType::Measures(measures) => {
                         rsx!{
                             Property {

diff --git a/crates/hooks/tests/use_editable.rs b/crates/hooks/tests/use_editable.rs
@@ -355,7 +355,7 @@ pub async fn highlight_multiple_lines_single_editor() {
     utils.wait_for_update().await;
 
     // Move cursor
-    utils.move_cursor((80., 20.)).await;
+    utils.move_cursor((80., 25.)).await;
 
     utils.wait_for_update().await;
 
@@ -849,7 +849,7 @@ pub async fn highlight_shift_click_multiple_lines_single_editor() {
     utils.wait_for_update().await;
 
     // Move and click cursor
-    utils.click_cursor((80., 20.)).await;
+    utils.click_cursor((80., 25.)).await;
 
     utils.wait_for_update().await;
 

diff --git a/crates/state/src/font_style.rs b/crates/state/src/font_style.rs
@@ -38,7 +38,7 @@ pub struct FontStyleState {
     pub font_slant: Slant,
     pub font_weight: Weight,
     pub font_width: Width,
-    pub line_height: f32, // https://developer.mozilla.org/en-US/docs/Web/CSS/line-height,
+    pub line_height: Option<f32>,
     pub decoration: Decoration,
     pub word_spacing: f32,
     pub letter_spacing: f32,
@@ -48,12 +48,7 @@ pub struct FontStyleState {
 }
 
 impl FontStyleState {
-    pub fn text_style(
-        &self,
-        default_font_family: &[String],
-        scale_factor: f32,
-        height_override: bool,
-    ) -> TextStyle {
+    pub fn text_style(&self, default_font_family: &[String], scale_factor: f32) -> TextStyle {
         let mut text_style = TextStyle::new();
         let mut font_family = self.font_family.clone();
 
@@ -69,9 +64,11 @@ impl FontStyleState {
             .set_font_size(self.font_size * scale_factor)
             .set_font_families(&font_family)
             .set_word_spacing(self.word_spacing)
-            .set_letter_spacing(self.letter_spacing)
-            .set_height_override(height_override)
-            .set_height(self.line_height);
+            .set_letter_spacing(self.letter_spacing);
+
+        if let Some(line_height) = self.line_height {
+            text_style.set_height_override(true).set_height(line_height);
+        }
 
         for text_shadow in self.text_shadows.iter() {
             text_style.add_shadow(*text_shadow);
@@ -95,7 +92,7 @@ impl Default for FontStyleState {
             font_weight: Weight::NORMAL,
             font_slant: Slant::Upright,
             font_width: Width::NORMAL,
-            line_height: 1.2,
+            line_height: None,
             word_spacing: 0.0,
             letter_spacing: 0.0,
             decoration: Decoration {
@@ -151,7 +148,7 @@ impl ParseAttribute for FontStyleState {
             AttributeName::LineHeight => {
                 if let Some(value) = attr.value.as_text() {
                     if let Ok(line_height) = value.parse::<f32>() {
-                        self.line_height = line_height.max(1.0);
+                        self.line_height = Some(line_height);
                     }
                 }
             }

diff --git a/examples/input.rs b/examples/input.rs
@@ -18,6 +18,7 @@ fn app() -> Element {
             padding: "7",
             width: "100%",
             height: "100%",
+            font_size: "10",
             label {
                 color: "black",
                 "Your name:"
@@ -38,9 +39,12 @@ fn app() -> Element {
                     values.write().1 = txt;
                 }
             },
-            label {
-                color: "black",
-                "You are {values.read().0} and you are {values.read().1} years old."
+            rect {
+                background: "red",
+                label {
+                    color: "black",
+                    "You are {values.read().0} and you are {values.read().1} years old."
+                }
             }
         }
     )