From ed0dc2ff72885dbe0f6e2fd135b612680e3e76fd Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 3 Sep 2023 11:07:38 +0200 Subject: [PATCH] LibWeb: Use flex layout for button content alignment Using flex layout inside button solves the issue with wrongly calculated height when it has: pseudo element and whitespaces inside. Also using flex instead of a table layout allows for the same vertical alignment but with fewer layout nodes: a flex container and anonymous wrapper for content instead of a table wrapper, table, row, and cell. --- .../button-baseline-align.txt | 22 +++---- .../block-and-inline/button-image-only.txt | 26 ++++----- ...should-have-vertically-aligned-content.txt | 58 +++++++++---------- .../button-with-abspos-pseudo-element.txt | 19 ++++++ ...tton-with-block-content-baseline-align.txt | 36 +++++------- ...on-with-multiple-words-text-node-label.txt | 22 +++---- ...ton-with-text-node-label-and-font-size.txt | 22 +++---- .../button-with-text-node-label.txt | 22 +++---- .../table/table-cell-not-paintable.txt | 10 +--- .../button-with-abspos-pseudo-element.html | 16 +++++ .../Libraries/LibWeb/Layout/TreeBuilder.cpp | 39 ++++++------- 11 files changed, 145 insertions(+), 147 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/button-with-abspos-pseudo-element.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/button-with-abspos-pseudo-element.html diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/button-baseline-align.txt b/Tests/LibWeb/Layout/expected/block-and-inline/button-baseline-align.txt index 9324050bde..af8b27f7eb 100644 --- a/Tests/LibWeb/Layout/expected/block-and-inline/button-baseline-align.txt +++ b/Tests/LibWeb/Layout/expected/block-and-inline/button-baseline-align.txt @@ -18,15 +18,13 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline TextNode <#text> BlockContainer diff --git a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp index 883b62add3..06c4e34401 100644 --- a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp +++ b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp @@ -383,25 +383,20 @@ ErrorOr TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder:: auto& parent = *dom_node.layout_node(); // If the box does not overflow in the vertical axis, then it is centered vertically. - auto table_computed_values = parent.computed_values().clone_inherited_values(); - static_cast(table_computed_values).set_display(CSS::Display::from_short(CSS::Display::Short::Table)); - static_cast(table_computed_values).set_height(CSS::Size::make_percentage(CSS::Percentage(100))); + // FIXME: Only apply alignment when box overflows + auto flex_computed_values = parent.computed_values().clone_inherited_values(); + auto& mutable_flex_computed_values = static_cast(flex_computed_values); + mutable_flex_computed_values.set_display(CSS::Display { CSS::Display::Outside::Block, CSS::Display::Inside::Flex }); + mutable_flex_computed_values.set_justify_content(CSS::JustifyContent::Center); + mutable_flex_computed_values.set_flex_direction(CSS::FlexDirection::Column); + mutable_flex_computed_values.set_height(CSS::Size::make_percentage(CSS::Percentage(100))); + auto flex_wrapper = parent.heap().template allocate_without_realm(parent.document(), nullptr, move(flex_computed_values)); - auto cell_computed_values = parent.computed_values().clone_inherited_values(); - static_cast(cell_computed_values).set_display(CSS::Display { CSS::Display::Internal::TableCell }); - static_cast(cell_computed_values).set_vertical_align(CSS::VerticalAlign::Middle); - - auto row_computed_values = parent.computed_values().clone_inherited_values(); - static_cast(row_computed_values).set_display(CSS::Display { CSS::Display::Internal::TableRow }); - - auto table_wrapper = parent.heap().template allocate_without_realm(parent.document(), nullptr, move(table_computed_values)); - auto cell_wrapper = parent.heap().template allocate_without_realm(parent.document(), nullptr, move(cell_computed_values)); - auto row_wrapper = parent.heap().template allocate_without_realm(parent.document(), nullptr, move(row_computed_values)); - - cell_wrapper->set_line_height(parent.line_height()); - cell_wrapper->set_font(parent.font()); - cell_wrapper->set_children_are_inline(parent.children_are_inline()); - row_wrapper->set_children_are_inline(false); + auto content_box_computed_values = parent.computed_values().clone_inherited_values(); + auto content_box_wrapper = parent.heap().template allocate_without_realm(parent.document(), nullptr, move(content_box_computed_values)); + content_box_wrapper->set_font(parent.font()); + content_box_wrapper->set_line_height(parent.line_height()); + content_box_wrapper->set_children_are_inline(parent.children_are_inline()); Vector> sequence; for (auto child = parent.first_child(); child; child = child->next_sibling()) { @@ -410,12 +405,12 @@ ErrorOr TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder:: for (auto& node : sequence) { parent.remove_child(*node); - cell_wrapper->append_child(*node); + content_box_wrapper->append_child(*node); } - row_wrapper->append_child(*cell_wrapper); - table_wrapper->append_child(*row_wrapper); - parent.append_child(*table_wrapper); + flex_wrapper->append_child(*content_box_wrapper); + + parent.append_child(*flex_wrapper); } return {};