Feat hide clear button (#174)

* Automatically toggle the clear button on the filterInput of the SubHeader component. * Simplify clearFilterButton implementation. * Add changeset * Add SubHeader component tests * Add mutation observer for finding the clear button * Move out the querySelector from the connectedCallback * Try disabling compatibility check * Fix linter issues * Enable Vite compatibility check * Address CR comments. * Make show_clear_button default to true * Fix tests * Remove unnecessary variable * Fix clear button flickering on page load * Simplify catalyst controller as only the class needs to be toggled to hide the clear button --------- Co-authored-by: Henriette Darge <h.darge@openproject.com>
opf · Sep 10, 2024 · 95adbb8 · 95adbb8
1 parent 75a39e1
commit 95adbb8
Show file tree

Hide file tree

Showing 9 changed files with 151 additions and 5 deletions.
diff --git a/.changeset/nice-adults-sniff.md b/.changeset/nice-adults-sniff.md
@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': minor
+---
+
+Toggle the visibility of the clear button on the SubHeader component's filter input based on its content.
diff --git a/app/components/primer/open_project/sub_header.pcss b/app/components/primer/open_project/sub_header.pcss
@@ -41,3 +41,7 @@
     width: 100%;
     gap: 8px;
 }
+
+.SubHeader-filterInput_hiddenClearButton + button.FormControl-input-trailingAction {
+  display: none;
+}
diff --git a/app/components/primer/open_project/sub_header.rb b/app/components/primer/open_project/sub_header.rb
@@ -38,7 +38,8 @@ class SubHeader < Primer::Component
       renders_one :filter_input, lambda { |name:, label:, **system_arguments|
         system_arguments[:classes] = class_names(
           system_arguments[:classes],
-          "SubHeader-filterInput"
+          "SubHeader-filterInput",
+          "SubHeader-filterInput_hiddenClearButton"
         )
         system_arguments[:placeholder] ||= I18n.t("button_filter")
         system_arguments[:leading_visual] ||= { icon: :search }
@@ -47,10 +48,25 @@ class SubHeader < Primer::Component
         system_arguments[:data] ||= {}
         system_arguments[:data][:target]= "sub-header.filterInput"
 
+        system_arguments[:show_clear_button] = true if system_arguments[:show_clear_button].nil?
+
+        if system_arguments[:show_clear_button]
+          system_arguments[:data] = merge_data(
+            system_arguments,
+            {
+              data: {
+                action: <<~JS
+                  input:sub-header#toggleFilterInputClearButton
+                  focus:sub-header#toggleFilterInputClearButton
+                JS
+              }
+            }
+          )
+        end
 
         @mobile_filter_trigger = Primer::Beta::IconButton.new(icon: system_arguments[:leading_visual][:icon],
                                                               display: [:inline_flex, :none],
-                                                              aria: {label: label },
+                                                              aria: { label: label },
                                                               "data-action": "click:sub-header#expandFilterInput",
                                                               "data-targets": HIDDEN_FILTER_TARGET_SELECTOR)
 

diff --git a/app/components/primer/open_project/sub_header_element.ts b/app/components/primer/open_project/sub_header_element.ts
@@ -2,10 +2,31 @@ import {controller, target, targets} from '@github/catalyst'
 
 @controller
 class SubHeaderElement extends HTMLElement {
-  @target filterInput: HTMLElement
+  @target filterInput: HTMLInputElement
   @targets hiddenItemsOnExpandedFilter: HTMLElement[]
   @targets shownItemsOnExpandedFilter: HTMLElement[]
 
+  connectedCallback() {
+    this.setupFilterInputClearButton()
+  }
+
+  setupFilterInputClearButton() {
+    this.waitForCondition(
+      () => Boolean(this.filterInput),
+      () => {
+        this.toggleFilterInputClearButton()
+      },
+    )
+  }
+
+  toggleFilterInputClearButton() {
+    if (this.filterInput.value.length > 0) {
+      this.filterInput.classList.remove('SubHeader-filterInput_hiddenClearButton')
+    } else {
+      this.filterInput.classList.add('SubHeader-filterInput_hiddenClearButton')
+    }
+  }
+
   expandFilterInput() {
     for (const item of this.hiddenItemsOnExpandedFilter) {
       item.classList.add('d-none')
@@ -31,6 +52,23 @@ class SubHeaderElement extends HTMLElement {
 
     this.classList.remove('SubHeader--expandedSearch')
   }
+
+  // Waits for condition to return true. If it returns false initially, this function creates a
+  // MutationObserver that calls body() whenever the contents of the component change.
+  private waitForCondition(condition: () => boolean, body: () => void) {
+    if (condition()) {
+      body()
+    } else {
+      const mutationObserver = new MutationObserver(() => {
+        if (condition()) {
+          body()
+          mutationObserver.disconnect()
+        }
+      })
+
+      mutationObserver.observe(this, {childList: true, subtree: true})
+    }
+  }
 }
 
 declare global {

diff --git a/previews/primer/open_project/sub_header_preview.rb b/previews/primer/open_project/sub_header_preview.rb
@@ -11,10 +11,23 @@ class SubHeaderPreview < ViewComponent::Preview
       # @param show_filter_input toggle
       # @param show_filter_button toggle
       # @param show_action_button toggle
+      # @param show_clear_button toggle
       # @param text text
-      def playground(show_filter_input: true, show_filter_button: true, show_action_button: true, text: nil)
+      # @param value text
+      def playground(
+        show_filter_input: true,
+        show_clear_button: true,
+        show_filter_button: true,
+        show_action_button: true,
+        text: nil,
+        value: nil
+      )
         render(Primer::OpenProject::SubHeader.new) do |component|
-          component.with_filter_input(name: "filter", label: "Filter") if show_filter_input
+          component.with_filter_input(
+            name: "filter",
+            label: "Filter",
+            show_clear_button: show_clear_button,
+            value: value) if show_filter_input
           component.with_filter_button do |button|
             button.with_trailing_visual_counter(count: "15")
             "Filter"

diff --git a/static/classes.json b/static/classes.json
@@ -591,6 +591,9 @@
   "SubHeader-filterContainer": [
     "Primer::OpenProject::SubHeader"
   ],
+  "SubHeader-filterInput_hiddenClearButton": [
+    "Primer::OpenProject::SubHeader"
+  ],
   "SubHeader-leftPane": [
     "Primer::OpenProject::SubHeader"
   ],

diff --git a/test/components/primer/open_project/sub_header_test.rb b/test/components/primer/open_project/sub_header_test.rb
@@ -92,4 +92,43 @@ def test_renders_a_custom_filter_button
     assert_selector(".SubHeader .MyCustomButton")
     assert_selector(".SubHeader .SubHeader-bottomPane .ABottomPane")
   end
+
+  def test_renders_a_clear_button_when_show_clear_button_is_set
+    render_inline(Primer::OpenProject::SubHeader.new) do |component|
+      component.with_filter_input(
+        name: "filter",
+        label: "Filter",
+        show_clear_button: true,
+        value: "value is set"
+      )
+    end
+
+    assert_selector(".SubHeader")
+    assert_selector(
+      ".SubHeader-filterInput"\
+      "[data-action=\"input:sub-header#toggleFilterInputClearButton\n"\
+                     "focus:sub-header#toggleFilterInputClearButton\n\"]"
+    )
+    assert_selector(".FormControl-input-trailingAction[data-action=\"click:primer-text-field#clearContents\"]")
+  end
+
+  def test_does_not_render_input_events_when_show_clear_button_is_not_set
+    render_inline(Primer::OpenProject::SubHeader.new) do |component|
+      component.with_filter_input(
+        name: "filter",
+        label: "Filter",
+        show_clear_button: false,
+        value: "value is set"
+      )
+    end
+
+    assert_selector(".SubHeader")
+    assert_selector(".SubHeader-filterInput")
+    assert_no_selector(
+      ".SubHeader-filterInput"\
+      "[data-action=\"input:sub-header#toggleFilterInputClearButton\n"\
+                     "focus:sub-header#toggleFilterInputClearButton\n\"]"
+    )
+    assert_no_selector(".FormControl-input-trailingAction[data-action=\"click:primer-text-field#clearContents\"]")
+  end
 end
diff --git a/test/css/component_specific_selectors_test.rb b/test/css/component_specific_selectors_test.rb
@@ -185,6 +185,7 @@ class ComponentSpecificSelectorsTest < Minitest::Test
     ],
     Primer::OpenProject::SubHeader => [
       ".SubHeader--expandedSearch",
+      ".SubHeader-filterInput_hiddenClearButton"
     ]
   }.freeze
 

diff --git a/test/system/open_project/sub_header_test.rb b/test/system/open_project/sub_header_test.rb
@@ -8,4 +8,31 @@ def test_renders_component
 
     assert_selector(".SubHeader")
   end
+
+  def render_clear_button_with_an_initial_value
+    visit_preview(:playground, show_clear_button: true, value: "value")
+
+    assert_selector(".FormControl-input-wrap--trailingAction")
+    assert_selector("button.FormControl-input-trailingAction")
+  end
+
+  def test_clear_button_functionality
+    visit_preview(:playground, show_clear_button: true)
+    # no clear button with empty value
+    assert_no_selector("button.FormControl-input-trailingAction")
+
+    fill_in "filter", with: "value"
+
+    # Clear the field with the "x" button
+    find(".FormControl-input-trailingAction").click
+    assert_no_selector("button.FormControl-input-trailingAction")
+
+    fill_in "filter", with: "value"
+    assert_selector("button.FormControl-input-trailingAction")
+
+    # Clear the field with backspace
+    filter_field = find_field "filter"
+    "value".length.times { filter_field.send_keys [:backspace] }
+    assert_no_selector("button.FormControl-input-trailingAction")
+  end
 end