diff --git a/.changeset/great-forks-work.md b/.changeset/great-forks-work.md new file mode 100644 index 0000000000..c82104df34 --- /dev/null +++ b/.changeset/great-forks-work.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Fix an accessibility issue where the Dialog body could not be reached via keyboard navigation diff --git a/app/components/primer/alpha/dialog.html.erb b/app/components/primer/alpha/dialog.html.erb index 67414761c6..ee7b921bac 100644 --- a/app/components/primer/alpha/dialog.html.erb +++ b/app/components/primer/alpha/dialog.html.erb @@ -5,7 +5,9 @@ <% if content.present? %> <%= content %> <% else %> - <%= body %> + + <%= body %> + <%= footer %> <% end %> <% end %> diff --git a/app/components/primer/alpha/dialog.rb b/app/components/primer/alpha/dialog.rb index bae650b7a9..58ebf5afd3 100644 --- a/app/components/primer/alpha/dialog.rb +++ b/app/components/primer/alpha/dialog.rb @@ -132,7 +132,7 @@ def initialize( @system_arguments, { aria: { disabled: true, - labelledby: "#{@id}-title", + labelledby: labelledby, describedby: "#{@id}-description" } } @@ -154,6 +154,10 @@ def before_render with_header unless header? with_body unless body? end + + def labelledby + "#{@id}-title" + end end end end diff --git a/app/components/primer/primer.ts b/app/components/primer/primer.ts index 6bdf81e1ce..8a22eb0fa4 100644 --- a/app/components/primer/primer.ts +++ b/app/components/primer/primer.ts @@ -4,6 +4,7 @@ import './alpha/dropdown' import './anchored_position' import './dialog_helper' import './focus_group' +import './scrollable_region' import './alpha/image_crop' import './beta/nav_list' import './beta/nav_list_group_element' diff --git a/app/components/primer/scrollable_region.ts b/app/components/primer/scrollable_region.ts new file mode 100644 index 0000000000..e18257ef3e --- /dev/null +++ b/app/components/primer/scrollable_region.ts @@ -0,0 +1,48 @@ +import {controller, attr} from '@github/catalyst' + +@controller +export class ScrollableRegionElement extends HTMLElement { + @attr hasOverflow = false + @attr labelledBy = '' + + observer: ResizeObserver + + connectedCallback() { + this.style.overflow = 'auto' + + this.observer = new ResizeObserver(entries => { + for (const entry of entries) { + this.hasOverflow = + entry.target.scrollHeight > entry.target.clientHeight || entry.target.scrollWidth > entry.target.clientWidth + } + }) + + this.observer.observe(this) + } + + disconnectedCallback() { + this.observer.disconnect() + } + + attributeChangedCallback(name: string) { + if (name === 'data-has-overflow') { + if (this.hasOverflow) { + this.setAttribute('aria-labelledby', this.labelledBy) + this.setAttribute('role', 'region') + this.setAttribute('tabindex', '0') + } else { + this.removeAttribute('aria-labelledby') + this.removeAttribute('role') + this.removeAttribute('tabindex') + } + } + } +} + +declare global { + interface Window { + ScrollableRegionElement: typeof ScrollableRegionElement + } +} + +window.ScrollableRegionElement = ScrollableRegionElement diff --git a/test/system/alpha/dialog_test.rb b/test/system/alpha/dialog_test.rb index 236efd4356..35c756b4d4 100644 --- a/test/system/alpha/dialog_test.rb +++ b/test/system/alpha/dialog_test.rb @@ -67,5 +67,12 @@ def test_closes_dialog_that_is_not_top_level assert_selector "dialog#dialog-one", visible: :hidden end + + # We're just opening the dialog with a scrollable body + # so the Axe scan can ensure the scrollable region is compliant + def test_with_scrollable_body + visit_preview(:long_text) + click_button("Show Dialog") + end end end