Skip to content

Commit

Permalink
Generate IDs for HTML in a standard way
Browse files Browse the repository at this point in the history
Tooltips were getting IDs that would occasionally conflict in CI. In
order to prevent that, we are standardizing our ID generation to start
with the component class name plus a random UUID.

Co-authored-by: Cameron Dutro <camertron@gmail.com>
  • Loading branch information
neall and camertron committed Jan 18, 2023
1 parent fdd7bc1 commit 4c17893
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-tigers-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Standardize how we generate ids for HTML
2 changes: 1 addition & 1 deletion app/components/primer/alpha/action_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def initialize(
show_dividers: false,
**system_arguments
)
@id = "action-list-#{SecureRandom.uuid}"
@id = self.class.generate_id
@role = role

@system_arguments = system_arguments
Expand Down
2 changes: 1 addition & 1 deletion app/components/primer/alpha/action_list/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def initialize(
description_scheme: DEFAULT_DESCRIPTION_SCHEME,
active: false,
on_click: nil,
id: SecureRandom.hex,
id: self.class.generate_id,
**system_arguments
)
@list = list
Expand Down
4 changes: 2 additions & 2 deletions app/components/primer/alpha/dialog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ def initialize(
position: DEFAULT_POSITION,
position_narrow: DEFAULT_POSITION_NARROW,
visually_hide_title: false,
id: "dialog-#{(36**3 + rand(36**4)).to_s(36)}",
id: self.class.generate_id,
**system_arguments
)
@system_arguments = deny_tag_argument(**system_arguments)

@system_arguments[:tag] = "modal-dialog"
@system_arguments[:role] = "dialog"
@system_arguments[:id] = id.to_s
@system_arguments[:id] = id
@system_arguments[:aria] = { modal: true }
@system_arguments[:classes] = class_names(
"Overlay",
Expand Down
4 changes: 0 additions & 4 deletions app/components/primer/alpha/tool_tip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,6 @@ class ToolTipElement extends HTMLElement {
this.hiddenFromView = true
this.#allowUpdatePosition = true

if (!this.id) {
this.id = `tooltip-${Date.now()}-${(Math.random() * 10000).toFixed(0)}`
}

if (!this.control) return

this.setAttribute('role', 'tooltip')
Expand Down
7 changes: 4 additions & 3 deletions app/components/primer/alpha/tooltip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,11 @@ def initialize(type:, for_id:, text:, direction: DIRECTION_DEFAULT, **system_arg

@text = text
@system_arguments = system_arguments
@system_arguments[:id] ||= self.class.generate_id
@system_arguments[:tag] = :"tool-tip"
@system_arguments[:for] = for_id
system_arguments[:classes] = class_names(
system_arguments[:classes],
@system_arguments[:classes] = class_names(
@system_arguments[:classes],
"sr-only"
)
@system_arguments[:position] = :absolute
Expand All @@ -121,6 +122,6 @@ def initialize(type:, for_id:, text:, direction: DIRECTION_DEFAULT, **system_arg
def call
render(Primer::BaseComponent.new(**@system_arguments)) { @text }
end
end
end
end
end
2 changes: 1 addition & 1 deletion app/components/primer/beta/icon_button.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def initialize(icon:, scheme: DEFAULT_SCHEME, wrapper_arguments: {}, show_toolti
@wrapper_arguments = wrapper_arguments
@show_tooltip = show_tooltip
@system_arguments = system_arguments
@system_arguments[:id] ||= "icon-button-#{SecureRandom.hex(4)}"
@system_arguments[:id] ||= self.class.generate_id

@system_arguments[:classes] = class_names(
"Button",
Expand Down
4 changes: 4 additions & 0 deletions app/components/primer/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def self.deprecated?
status == :deprecated
end

def self.generate_id
"#{name.demodulize.underscore.dasherize}-#{SecureRandom.uuid}"
end

private

def raise_on_invalid_options?
Expand Down
2 changes: 1 addition & 1 deletion app/components/primer/icon_button.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def initialize(icon:, scheme: DEFAULT_SCHEME, box: false, tooltip_direction: Pri

@system_arguments = system_arguments

@system_arguments[:id] ||= "icon-button-#{SecureRandom.hex(4)}"
@system_arguments[:id] ||= self.class.generate_id

@system_arguments[:classes] = class_names(
"btn-octicon",
Expand Down
2 changes: 1 addition & 1 deletion lib/primer/forms/dsl/input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def initialize(builder:, form:, **system_arguments)

@input_arguments[:invalid] = "true" if invalid?

base_id = SecureRandom.hex[0..5]
base_id = SecureRandom.uuid

@ids = {}.tap do |id_map|
id_map[:validation] = "validation-#{base_id}" if invalid?
Expand Down

0 comments on commit 4c17893

Please sign in to comment.