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 8ebded10ce..dec087b0d4 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 e5efdc842f..65f7a27017 100644 --- a/app/components/primer/alpha/dialog.rb +++ b/app/components/primer/alpha/dialog.rb @@ -133,7 +133,7 @@ def initialize( @system_arguments, { aria: { disabled: true, - labelledby: "#{@id}-title", + labelledby: labelledby, describedby: "#{@id}-description" } } @@ -155,6 +155,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 0a8fc194af..a730255725 100644 --- a/app/components/primer/primer.ts +++ b/app/components/primer/primer.ts @@ -3,6 +3,7 @@ import './alpha/action_bar_element' import './alpha/dropdown' import './anchored_position' import './focus_group' +import './scrollable_region' import './alpha/image_crop' import './alpha/modal_dialog' import './beta/nav_list' 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 336c82d5a3..fbafcb08a2 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 "modal-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