From c30911ab102145117a551ee8b5daf50900636e66 Mon Sep 17 00:00:00 2001
From: Bastiaan van der Plaat <bastiaan.v.d.plaat@gmail.com>
Date: Wed, 20 Dec 2023 18:21:32 +0100
Subject: [PATCH] LibWeb: Hide select chevron icon when `appearance: none;`

---
 Base/res/html/misc/select.html                | 21 +++++++++++++++++
 Userland/Libraries/LibWeb/CSS/Default.css     |  4 ----
 Userland/Libraries/LibWeb/DOM/Element.cpp     |  2 ++
 Userland/Libraries/LibWeb/DOM/Element.h       |  2 ++
 .../LibWeb/HTML/HTMLSelectElement.cpp         | 23 +++++++++++++++----
 .../Libraries/LibWeb/HTML/HTMLSelectElement.h |  3 +++
 6 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/Base/res/html/misc/select.html b/Base/res/html/misc/select.html
index 2618a21f75f..4d1f8de676a 100644
--- a/Base/res/html/misc/select.html
+++ b/Base/res/html/misc/select.html
@@ -4,6 +4,16 @@
     <meta charset="UTF-8">
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
     <title>Select showcase</title>
+    <style>
+        .custom {
+            border: 1px solid rgba(0, 0, 0, 0.5);
+            border-radius: 8px;
+            appearance: none;
+            padding: 8px 16px;
+            background-color: #088;
+            color: #fff;
+        }
+    </style>
 </head>
 <body>
     <p>Basic select:</p>
@@ -33,6 +43,17 @@
         Value: <span id="b-value">?</span>
     </p>
 
+    <p>Basic select with custom styling:</p>
+    <p>
+        <select class="custom">
+            <option value="one">One</option>
+            <option value="two">Two</option>
+            <option value="three">Three</option>
+            <option value="four">Four</option>
+            <option value="five">Five</option>
+        </select>
+    </p>
+
     <p>Basic select with option groups and separators:</p>
     <p>
         <select onchange="document.getElementById('c-value').textContent = this.value">
diff --git a/Userland/Libraries/LibWeb/CSS/Default.css b/Userland/Libraries/LibWeb/CSS/Default.css
index 8e3d3f67e51..55db64d477e 100644
--- a/Userland/Libraries/LibWeb/CSS/Default.css
+++ b/Userland/Libraries/LibWeb/CSS/Default.css
@@ -62,10 +62,6 @@ button:hover, input[type=submit]:hover, input[type=button]:hover, input[type=res
     background-color: #e5e0d7;
 }
 
-select {
-    padding-right: 2px;
-}
-
 option {
     display: none;
 }
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index 39046cf8773..43822050ef1 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -573,6 +573,7 @@ Element::RequiredInvalidationAfterStyleChange Element::recompute_style()
         return invalidation;
 
     m_computed_css_values = move(new_computed_css_values);
+    computed_css_values_changed();
 
     if (!invalidation.rebuild_layout_tree && layout_node()) {
         // If we're keeping the layout tree, we can just apply the new style to the existing layout tree.
@@ -2043,6 +2044,7 @@ size_t Element::attribute_list_size() const
 void Element::set_computed_css_values(RefPtr<CSS::StyleProperties> style)
 {
     m_computed_css_values = move(style);
+    computed_css_values_changed();
 }
 
 auto Element::pseudo_element_custom_properties() const -> PseudoElementCustomProperties&
diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h
index 8b0a6d0f873..d1201753524 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.h
+++ b/Userland/Libraries/LibWeb/DOM/Element.h
@@ -386,6 +386,8 @@ protected:
     virtual void children_changed() override;
     virtual i32 default_tab_index_value() const;
 
+    virtual void computed_css_values_changed() { }
+
     virtual void visit_edges(Cell::Visitor&) override;
 
     virtual bool id_reference_exists(String const&) const override;
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
index 3b61ce936e6..4b72b009263 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
@@ -45,6 +45,7 @@ void HTMLSelectElement::visit_edges(Cell::Visitor& visitor)
     Base::visit_edges(visitor);
     visitor.visit(m_options);
     visitor.visit(m_inner_text_element);
+    visitor.visit(m_chevron_icon_element);
 }
 
 JS::GCPtr<Layout::Node> HTMLSelectElement::create_layout_node(NonnullRefPtr<CSS::StyleProperties> style)
@@ -332,6 +333,19 @@ void HTMLSelectElement::form_associated_element_was_removed(DOM::Node*)
     set_shadow_root(nullptr);
 }
 
+void HTMLSelectElement::computed_css_values_changed()
+{
+    // Hide chevron icon when appearance is none
+    if (m_chevron_icon_element) {
+        auto appearance = computed_css_values()->appearance();
+        if (appearance.has_value() && *appearance == CSS::Appearance::None) {
+            MUST(m_chevron_icon_element->style_for_bindings()->set_property(CSS::PropertyID::Display, "none"_string));
+        } else {
+            MUST(m_chevron_icon_element->style_for_bindings()->set_property(CSS::PropertyID::Display, "block"_string));
+        }
+    }
+}
+
 void HTMLSelectElement::create_shadow_tree_if_needed()
 {
     if (shadow_root_internal())
@@ -355,14 +369,14 @@ void HTMLSelectElement::create_shadow_tree_if_needed()
     MUST(border->append_child(*m_inner_text_element));
 
     // FIXME: Find better way to add chevron icon
-    auto chevron_icon_element = DOM::create_element(document(), HTML::TagNames::div, Namespace::HTML).release_value_but_fixme_should_propagate_errors();
-    MUST(chevron_icon_element->set_attribute(HTML::AttributeNames::style, R"~~~(
+    m_chevron_icon_element = DOM::create_element(document(), HTML::TagNames::div, Namespace::HTML).release_value_but_fixme_should_propagate_errors();
+    MUST(m_chevron_icon_element->set_attribute(HTML::AttributeNames::style, R"~~~(
         width: 16px;
         height: 16px;
         margin-left: 4px;
     )~~~"_string));
-    MUST(chevron_icon_element->set_inner_html("<svg xmlns=\"http://www.w3.org/2000/svg\" viewBox=\"0 0 24 24\"><path d=\"M7.41,8.58L12,13.17L16.59,8.58L18,10L12,16L6,10L7.41,8.58Z\" /></svg>"sv));
-    MUST(border->append_child(*chevron_icon_element));
+    MUST(m_chevron_icon_element->set_inner_html("<svg xmlns=\"http://www.w3.org/2000/svg\" viewBox=\"0 0 24 24\"><path d=\"M7.41,8.58L12,13.17L16.59,8.58L18,10L12,16L6,10L7.41,8.58Z\" /></svg>"sv));
+    MUST(border->append_child(*m_chevron_icon_element));
 
     update_inner_text_element();
 }
@@ -377,5 +391,4 @@ void HTMLSelectElement::update_inner_text_element()
         }
     }
 }
-
 }
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h
index d57914783a5..ecdbe9e37ff 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h
+++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h
@@ -89,12 +89,15 @@ private:
     // ^DOM::Element
     virtual i32 default_tab_index_value() const override;
 
+    virtual void computed_css_values_changed() override;
+
     void create_shadow_tree_if_needed();
     void update_inner_text_element();
 
     JS::GCPtr<HTMLOptionsCollection> m_options;
     bool m_is_open { false };
     JS::GCPtr<DOM::Element> m_inner_text_element;
+    JS::GCPtr<DOM::Element> m_chevron_icon_element;
 };
 
 }
