Revert "Use visibility: 'hidden' instead of hidden attribute" (#1295

)

* Revert "Use `visibility: 'hidden'` instead of `hidden` attribute (#1256)"

This reverts commit 544dd56.

* Create red-wolves-notice.md

.changeset/red-wolves-notice.md

@@ -0,0 +1,5 @@
+---
+"@primer/view-components": patch
+---
+
+Revert "Use `visibility: 'hidden'` instead of `hidden` attribute"

app/components/primer/alpha/tool-tip-element.ts

@@ -176,14 +176,6 @@ class ToolTipElement extends HTMLElement {
     return this.ownerDocument.getElementById(this.htmlFor)
   }
 
-  set visibilityHidden(value: true | false) {
-    this.style.visibility = value ? 'hidden' : ''
-  }
-
-  get visibilityHidden() {
-    return this.style.visibility === 'hidden'
-  }
-
   connectedCallback() {
     if (!this.shadowRoot) {
       const shadow = this.attachShadow({mode: 'open'})
@@ -195,7 +187,7 @@ class ToolTipElement extends HTMLElement {
         <slot></slot>
       `
     }
-    this.visibilityHidden = true
+    this.hidden = true
     this.#allowUpdatePosition = true
 
     if (!this.id) {
@@ -228,36 +220,36 @@ class ToolTipElement extends HTMLElement {
 
     // Ensures that tooltip stays open when hovering between tooltip and element
     // WCAG Success Criterion 1.4.13 Hoverable
-    if ((event.type === 'mouseenter' || event.type === 'focus') && this.visibilityHidden) {
-      this.visibilityHidden = false
+    if ((event.type === 'mouseenter' || event.type === 'focus') && this.hidden) {
+      this.hidden = false
     } else if (event.type === 'blur') {
-      this.visibilityHidden = true
+      this.hidden = true
     } else if (
       event.type === 'mouseleave' &&
       (event as MouseEvent).relatedTarget !== this.control &&
       (event as MouseEvent).relatedTarget !== this
     ) {
-      this.visibilityHidden = true
-    } else if (event.type === 'keydown' && (event as KeyboardEvent).key === 'Escape' && !this.visibilityHidden) {
-      this.visibilityHidden = true
+      this.hidden = true
+    } else if (event.type === 'keydown' && (event as KeyboardEvent).key === 'Escape' && !this.hidden) {
+      this.hidden = true
     }
   }
 
-  static observedAttributes = ['data-type', 'data-direction', 'id', 'style']
+  static observedAttributes = ['data-type', 'data-direction', 'id', 'hidden']
 
   #update() {
-    if (this.visibilityHidden) {
+    if (this.hidden) {
       this.classList.remove(TOOLTIP_OPEN_CLASS, ...DIRECTION_CLASSES)
     } else {
       this.classList.add(TOOLTIP_OPEN_CLASS)
-      for (const tooltip of this.ownerDocument.querySelectorAll<ToolTipElement>(this.tagName)) {
-        if (tooltip !== this) tooltip.visibilityHidden = true
+      for (const tooltip of this.ownerDocument.querySelectorAll<HTMLElement>(this.tagName)) {
+        if (tooltip !== this) tooltip.hidden = true
       }
       this.#updatePosition()
     }
   }
 
-  attributeChangedCallback(name: string, oldValue: string, newValue: string) {
+  attributeChangedCallback(name: string) {
     if (name === 'id' || name === 'data-type') {
       if (!this.id || !this.control) return
       if (this.type === 'label') {
@@ -268,7 +260,7 @@ class ToolTipElement extends HTMLElement {
         describedBy ? (describedBy = `${describedBy} ${this.id}`) : (describedBy = this.id)
         this.control.setAttribute('aria-describedby', describedBy)
       }
-    } else if (this.isConnected && name === 'style' && (oldValue && oldValue.includes('visibility')) || (newValue && newValue.includes('visibility'))) {
+    } else if (this.isConnected && name === 'hidden') {
       this.#update()
     } else if (name === 'data-direction') {
       this.classList.remove(...DIRECTION_CLASSES)
@@ -303,7 +295,7 @@ class ToolTipElement extends HTMLElement {
 
   #updatePosition() {
     if (!this.control) return
-    if (!this.#allowUpdatePosition || this.visibilityHidden) return
+    if (!this.#allowUpdatePosition || this.hidden) return
 
     const TOOLTIP_OFFSET = 10
 

app/components/primer/alpha/tooltip.rb

@@ -88,8 +88,8 @@ def initialize(type:, for_id:, text:, direction: DIRECTION_DEFAULT, **system_arg
 
         @text = text
         @system_arguments = system_arguments
+        @system_arguments[:hidden] = true
         @system_arguments[:tag] = :"tool-tip"
-        @system_arguments[:style] = join_style_arguments(@system_arguments[:style], "visibility: hidden")
         @system_arguments[:for] = for_id
         @system_arguments[:position] = :absolute
         @system_arguments[:"data-direction"] = fetch_or_fallback(DIRECTION_OPTIONS, direction, DIRECTION_DEFAULT).to_s

app/lib/primer/join_style_arguments_helper.rb

@@ -8,7 +8,7 @@ module JoinStyleArgumentsHelper
     # join_style_arguments("width: 100%", "height: 100%") =>
     #   "width: 100%;height: 100%"
     def join_style_arguments(*args)
-      args.compact.map { |a| a.strip.chomp(";") }.join(";")
+      args.compact.join(";")
     end
   end
 end

test/components/beta/truncate_test.rb

@@ -64,6 +64,6 @@ def test_truncate_with_max_width
       component.item(max_width: 1337) { "content" }
     end
 
-    assert_selector(".Truncate > .Truncate-text[style='max-width: 1337px']", text: "content")
+    assert_selector(".Truncate > .Truncate-text[style='max-width: 1337px;']", text: "content")
   end
 end

test/components/component_test.rb

@@ -120,8 +120,8 @@ def test_all_components_pass_through_classes
   def test_all_components_support_inline_styles
     default_args = { style: "width: 100%;" }
     COMPONENTS_WITH_ARGS.each do |component, args, proc|
-      rendered = render_component(component, default_args.merge(args), proc)
-      assert_equal(true, rendered.inner_html.include?('style="width: 100%;'))
+      render_component(component, default_args.merge(args), proc)
+      assert_selector("[style='width: 100%;']", visible: :all)
     end
   end
 

test/components/progress_bar_component_test.rb

@@ -38,15 +38,15 @@ def test_renders_percent_completed_progress
       component.item(percentage: 80)
     end
 
-    assert_selector("[style='width: 80%']")
+    assert_selector("[style='width: 80%;']")
   end
 
   def test_renders_custom_styles
     render_inline(Primer::ProgressBarComponent.new) do |component|
       component.item(percentage: 80, style: "color: red")
     end
 
-    assert_selector("[style='color: red;width: 80%']")
+    assert_selector("[style='color: red;width: 80%;']")
   end
 
   def test_renders_background_colors

test/components/truncate_test.rb

@@ -49,6 +49,6 @@ def test_does_not_render_expandable_when_not_inline
   def test_renders_custom_max_width
     render_inline(Primer::Truncate.new(max_width: 100)) { "content" }
 
-    assert_selector(".css-truncate", text: "content", style: "max-width: 100px")
+    assert_selector(".css-truncate", text: "content", style: "max-width: 100px;")
   end
 end

test/lib/join_style_arguments_helper_test.rb

@@ -30,11 +30,4 @@ def test_handles_nil
       join_style_arguments(nil, "width: 100%")
     )
   end
-
-  def test_handles_semicolon
-    assert_equal(
-      "width: 100%;background-color: pink",
-      join_style_arguments("width: 100%;", "background-color: pink")
-    )
-  end
 end