Replacing role=dialog div with dialog element by shaynabergeron · Pull Request #661 · la-ots/pelican

shaynabergeron · 2026-03-31T17:55:33Z

Replaced <div role="dialog"> with the native <dialog> element.

Using dialog because - built-in accessibility (focus, modal behavior, semantics)
Aligns with WCAG best practices
Added proper tab and keyboard functionality in JS
Removed acl public-read instances

Copilot

Pull request overview

This PR migrates the docs modal implementation from a <div role="dialog"> pattern to the native HTML <dialog> element, updating styles and client-side behavior accordingly, and removes explicit public-read ACL usage from docs S3 deploy workflows.

Changes:

Replaced modal markup to use <dialog class="modal"> and adjusted ARIA wiring.
Added custom JS to open/close dialogs, manage focus, and handle backdrop click/keyboard interactions.
Updated docs deployment workflows to remove aws s3 sync --acl public-read.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
scss/_modal.scss	Adds `<dialog>`-targeted modal styling to mimic existing modal layout.
docs/js/default.js	Implements dialog open/close behavior and keyboard/focus handling for dialogs.
docs/_includes/markup/modal.njk	Updates example modal markup to use `<dialog>` and updated close/label wiring.
.github/workflows/docs-vnext.yml	Removes `--acl public-read` from vNext docs S3 sync.
.github/workflows/docs-sandbox.yml	Removes `--acl public-read` from sandbox docs S3 sync.
.github/workflows/docs-production.yml	Removes `--acl public-read` from production docs S3 sync.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T20:48:41Z

+dialog.modal .modal-dialog {
+  width: auto;
+  max-width: 500px;
+  pointer-events: none;
+}


pointer-events: none on .modal-dialog disables all mouse/touch interactions for the dialog contents (buttons/links inside won’t be clickable, and clicks will likely be treated as “outside” clicks). Remove this rule or re-enable pointer events on the interactive container (e.g., .modal-content).

Copilot · 2026-03-31T20:48:42Z

+document.querySelectorAll('[data-toggle="modal"], [data-bs-toggle="modal"]').forEach((button) => {
+  button.addEventListener("click", (event) => {
+    event.preventDefault();
+
+    const targetSelector =
+      button.getAttribute("data-bs-target") ||
+      button.getAttribute("data-target");
+
+    if (!targetSelector) return;
+
+    const dialog = document.querySelector(targetSelector);
+    if (!(dialog instanceof HTMLDialogElement)) return;
+
+    dialog._invoker = button;
+    dialog.showModal();
+
+    setTimeout(() => {
+      const focusables = getFocusableElements(dialog);
+      if (focusables.length) focusables[0].focus();
+    }, 0);
+  });


This click handler will run alongside Bootstrap’s built-in Data API for [data-bs-toggle="modal"] (preventDefault doesn’t stop Bootstrap’s click listener). With Bootstrap loaded (tooltips/popovers are used above), the same click can trigger Bootstrap’s modal code which expects a .modal div and may throw or behave incorrectly. Consider removing the data(-bs)-toggle="modal" attributes for dialogs or calling event.stopImmediatePropagation() when opening a <dialog> to prevent Bootstrap handling.

Copilot · 2026-03-31T20:48:42Z

+  dialog.addEventListener("keydown", (event) => {
+    const focusables = getFocusableElements(dialog);
+    if (!focusables.length) return;
+
+    const currentIndex = focusables.indexOf(document.activeElement);
+
+    if (["ArrowRight", "ArrowDown"].includes(event.key)) {
+      event.preventDefault();
+      const nextIndex = (currentIndex + 1) % focusables.length;
+      focusables[nextIndex].focus();
+    }
+
+    if (["ArrowLeft", "ArrowUp"].includes(event.key)) {
+      event.preventDefault();
+      const prevIndex =
+        (currentIndex - 1 + focusables.length) % focusables.length;
+      focusables[prevIndex].focus();
+    }


The keydown handler prevents default for Arrow keys to cycle focus. This breaks expected keyboard behavior inside form fields (e.g., arrow keys moving the caret in inputs/textareas, changing selection in selects). If focus management is required, it should be limited to Tab/Shift+Tab (or only apply arrow handling when the active element is not an input/textarea/select/contenteditable).

Copilot · 2026-03-31T20:48:42Z

+    const focusables = getFocusableElements(dialog);
+    if (!focusables.length) return;
+
+    const currentIndex = focusables.indexOf(document.activeElement);
+
+    if (["ArrowRight", "ArrowDown"].includes(event.key)) {
+      event.preventDefault();
+      const nextIndex = (currentIndex + 1) % focusables.length;
+      focusables[nextIndex].focus();
+    }
+
+    if (["ArrowLeft", "ArrowUp"].includes(event.key)) {
+      event.preventDefault();
+      const prevIndex =
+        (currentIndex - 1 + focusables.length) % focusables.length;
+      focusables[prevIndex].focus();
+    }
+
+    if (event.key === "Tab") {
+      if (event.shiftKey && currentIndex === 0) {
+        event.preventDefault();
+        focusables[focusables.length - 1].focus();
+      } else if (!event.shiftKey && currentIndex === focusables.length - 1) {
+        event.preventDefault();
+        focusables[0].focus();
+      }
+    }


currentIndex can be -1 when document.activeElement isn’t in focusables (e.g., initial focus is on the dialog itself, or an element filtered out). In that case the Tab trapping logic won’t reliably keep focus in the dialog. Handle the -1 case explicitly (e.g., treat it as 0/last depending on Shift+Tab) before computing wraparound.

Copilot · 2026-03-31T20:48:42Z

+  id="ModalID"
+  class="modal"
+  aria-labelledby="ModalTitle"
+  aria-describedby="ModalBody"


This template previously configured a “static” backdrop (data(-bs)-backdrop="static"), but the new dialog implementation always closes on outside click (see dialog.addEventListener("click", ...) in default.js). If the intent is to preserve the static-backdrop behavior, add an opt-in/out mechanism (e.g., a data-backdrop="static" check) so dialogs don’t unexpectedly dismiss when clicking outside.

Suggested change

aria-describedby="ModalBody"

aria-describedby="ModalBody"

data-backdrop="static"

data-bs-backdrop="static"

Copilot · 2026-03-31T20:48:42Z

+        <form method="dialog" class="modal-footer-form">
+          <button type="button" class="btn btn-outline-ui" id="resetTimer" value="choice" data-dismiss="modal" data-bs-dismiss="modal">Choice</button>
          <button type="button" class="btn btn-primary" id="logout_url">Main Choice</button>
        </form>


<form method="dialog"> only closes the dialog when it’s submitted (e.g., via a type="submit" button). In this markup both buttons are type="button", so the method="dialog" (and the value="choice") won’t have any effect. Either switch the intended closing action to type="submit" (and rely on method="dialog"), or change the form back to a normal form/container to avoid misleading semantics.

Suggested change

<form method="dialog" class="modal-footer-form">

<button type="button" class="btn btn-outline-ui" id="resetTimer" value="choice" data-dismiss="modal" data-bs-dismiss="modal">Choice</button>

<button type="button" class="btn btn-primary" id="logout_url">Main Choice</button>

</form>

<div class="modal-footer-form">

<button type="button" class="btn btn-outline-ui" id="resetTimer" value="choice" data-dismiss="modal" data-bs-dismiss="modal">Choice</button>

<button type="button" class="btn btn-primary" id="logout_url">Main Choice</button>

</div>

Copilot · 2026-03-31T20:48:43Z

+
+    const dialog = document.querySelector(targetSelector);
+    if (!(dialog instanceof HTMLDialogElement)) return;
+


dialog.showModal() throws an InvalidStateError if the dialog is already open (e.g., double-clicking the trigger, or triggering it while open). Add a guard like if (dialog.open) return; (or close/reopen) to avoid uncaught exceptions breaking subsequent JS.

Suggested change

if (dialog.open) return;

james-alt · 2026-07-10T14:51:24Z

@shaynabergeron is this PR still relevant? If so, it has a change to the scss and would need to also bump the version number and include what's changed in the release notes.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

+function openModal() {
+  if (!(modal instanceof HTMLDialogElement)) return;
+  modal.showModal();
+


+  modal.addEventListener("click", (event) => {
+    if (event.target === modal) {
+      event.preventDefault();
+    }
+  });


  target.style.boxSizing = "border-box";
  target.style.height = target.offsetHeight + "px";
-  target.offsetHeight; // Force reflow
+  target.offsetHeight;


  target.style.marginTop = 0;
  target.style.marginBottom = 0;
-  target.offsetHeight; // Force reflow
+  target.offsetHeight;


+dialog.simple-modal:focus {
+  outline: none;
+}


+  align-items: flex-start;
+  justify-content: space-between;
+  padding: 1rem 1rem;
+  border-bottom: 1px solid #dee2e6;


+  display: flex;
+  justify-content: flex-end;
+  padding: 1rem;
+  border-top: 1px solid #dee2e6;


shaynabergeron added 2 commits March 31, 2026 12:52

replace role=dialog with tag

7b916bb

edit css

adcb2b9

shaynabergeron requested a review from isralduke March 31, 2026 17:55

shaynabergeron self-assigned this Mar 31, 2026

shaynabergeron requested review from a team as code owners March 31, 2026 17:55

shaynabergeron had a problem deploying to Docs (Sandbox) March 31, 2026 17:55 — with GitHub Actions Failure

shaynabergeron mentioned this pull request Mar 31, 2026

Improve Modal for A11y #660

Open

removing acl public read instance

d12c3ea

shaynabergeron requested a review from a team as a code owner March 31, 2026 18:06

shaynabergeron temporarily deployed to Docs (Sandbox) March 31, 2026 18:06 — with GitHub Actions Inactive

james-alt requested a review from Copilot March 31, 2026 20:44

Copilot started reviewing on behalf of james-alt March 31, 2026 20:45 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

make modal better

7d3ec0e

shaynabergeron temporarily deployed to Docs (Sandbox) April 8, 2026 20:52 — with GitHub Actions Inactive

james-alt requested a review from Copilot July 10, 2026 14:51

Copilot started reviewing on behalf of james-alt July 10, 2026 14:51 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replacing role=dialog div with dialog element#661

Replacing role=dialog div with dialog element#661
shaynabergeron wants to merge 4 commits into
mainfrom
making-modal-better

shaynabergeron commented Mar 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

james-alt commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		const dialog = document.querySelector(targetSelector);
		if (!(dialog instanceof HTMLDialogElement)) return;

Uh oh!

Conversation

shaynabergeron commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

james-alt commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shaynabergeron commented Mar 31, 2026 •

edited

Loading