Skip to content

Commit

Permalink
Merge upstream
Browse files Browse the repository at this point in the history
  • Loading branch information
camertron committed Feb 1, 2024
2 parents f3a8892 + cdd12be commit ab41339
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-meals-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Ensure Overlays that open dialogs do not close when the Dialog opens
14 changes: 14 additions & 0 deletions app/components/primer/dialog_helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ export class DialogHelperElement extends HTMLElement {
for (const record of records) {
if (record.target === this.dialog) {
this.ownerDocument.body.classList.toggle('has-modal', this.dialog.hasAttribute('open'))
// In some older browsers, such as Chrome 122, when a top layer element (such as a dialog)
// opens from within a popover, the "hide all popovers" internal algorithm runs, hiding
// any popover that is currently open, regardless of whether or not another top layer element,
// such as a <dialog> is nested inside.
// See https://github.com/whatwg/html/issues/9998.
// This is fixed by https://github.com/whatwg/html/pull/10116, but while we still support browsers that present this bug,
// we must undo the work they did to hide ancestral popovers of the dialog:
if (this.dialog.hasAttribute('open')) {
let node: HTMLElement | null = this.dialog
while (node) {
node = node.closest('[popover]:not(:popover-open)')
if (node) node.showPopover()
}
}
}
}
}).observe(this, {subtree: true, attributeFilter: ['open']})
Expand Down
23 changes: 23 additions & 0 deletions previews/primer/alpha/dialog_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,29 @@ def scroll_container(title: "Test Dialog", subtitle: nil, position: :center, siz
visually_hide_title: visually_hide_title
})
end

# @label Dialog inside Overlay
#
# @param title [String] text
# @param subtitle [String] text
# @param size [Symbol] select [small, medium, medium_portrait, large, xlarge]
# @param position [Symbol] select [center, right, left]
# @param position_narrow [Symbol] select [inherit, bottom, fullscreen, left, right]
# @param visually_hide_title [Boolean] toggle
# @param button_text [String] text
# @param body_text [String] text
def dialog_inside_overlay(title: "Test Dialog", subtitle: nil, position: :center, size: :medium, button_text: "Show Dialog", body_text: "Content", position_narrow: :fullscreen, visually_hide_title: false)
render_with_template(locals: {
title: title,
subtitle: subtitle,
position: position,
size: size,
button_text: button_text,
body_text: body_text,
position_narrow: position_narrow,
visually_hide_title: visually_hide_title
})
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<%= render(Primer::Alpha::Overlay.new(title: "An overlay")) do |o| %>
<% o.with_show_button() { "Show overlay" } %>
<% o.with_body() do %>
<%= render(Primer::Alpha::Dialog.new(id: "dialog-one", title: title, position: position, subtitle: subtitle, visually_hide_title: false)) do |d| %>
<% d.with_show_button { button_text } %>
<% d.with_body { body_text} %>
<% end %>
<% end %>
<% end %>
13 changes: 13 additions & 0 deletions static/info_arch.json
Original file line number Diff line number Diff line change
Expand Up @@ -3375,6 +3375,19 @@
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/dialog/dialog_inside_overlay",
"name": "dialog_inside_overlay",
"snapshot": "false",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
}
],
"subcomponents": [
Expand Down
13 changes: 13 additions & 0 deletions static/previews.json
Original file line number Diff line number Diff line change
Expand Up @@ -3139,6 +3139,19 @@
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/dialog/dialog_inside_overlay",
"name": "dialog_inside_overlay",
"snapshot": "false",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
}
]
},
Expand Down
8 changes: 8 additions & 0 deletions test/system/alpha/dialog_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,13 @@ def test_click_events_can_be_added_to_invoker_buttons

assert page.evaluate_script("window.dialogInvokerClicked"), "click event was not fired"
end

def test_dialog_inside_overlay_opens_when_clicked
visit_preview(:dialog_inside_overlay)

click_button("Show overlay")
click_button("Show Dialog")
assert_selector "dialog[open]"
end
end
end

0 comments on commit ab41339

Please sign in to comment.