Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: focus [autofocus] element in a popup #2736

Merged
merged 2 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cypress/e2e/accessibility.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,15 @@ describe('Focus', () => {
})
expect(document.activeElement).to.equal(Swal.getDenyButton())
})

it('[autofocus]', () => {
Swal.fire({
html: `
<a href>link 1</a>
<a href autofocus>link 2</a>
<a href autofocus>link 3</a>
`,
})
expect(document.activeElement.innerText).to.equal('link 2')
})
})
34 changes: 31 additions & 3 deletions src/SweetAlert.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,22 +221,50 @@ const setupTimer = (globalState, innerParams, dismissWith) => {
}

/**
* Initialize focus in the popup:
*
* 1. If `toast` is `true`, don't steal focus from the document.
* 2. Else if there is an [autofocus] element, focus it.
* 3. Else if `focusConfirm` is `true` and confirm button is visible, focus it.
* 4. Else if `focusDeny` is `true` and deny button is visible, focus it.
* 5. Else if `focusCancel` is `true` and cancel button is visible, focus it.
* 6. Else focus the first focusable element in a popup (if any).
*
* @param {DomCache} domCache
* @param {SweetAlertOptions} innerParams
*/
const initFocus = (domCache, innerParams) => {
if (innerParams.toast) {
return
}

// TODO: this is dumb, remove `allowEnterKey` param in the next major version
if (!callIfFunction(innerParams.allowEnterKey)) {
blurActiveElement()
return
}

if (!focusButton(domCache, innerParams)) {
setFocus(-1, 1)
if (focusAutofocus(domCache)) {
return
}

if (focusButton(domCache, innerParams)) {
return
}

setFocus(-1, 1)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor initFocus to improve readability and maintainability.

The initFocus function has several conditional checks to determine where the focus should be set. To improve readability and maintainability, consider restructuring this function to reduce nesting and clarify the logic flow:

const initFocus = (domCache, innerParams) => {
  if (innerParams.toast) return;

  if (!callIfFunction(innerParams.allowEnterKey)) {
    blurActiveElement();
    return;
  }

  const focusHandlers = [
    () => focusAutofocus(domCache),
    () => focusButton(domCache, innerParams)
  ];

  for (const handler of focusHandlers) {
    if (handler()) return;
  }

  setFocus(-1, 1);
}

This refactoring uses an array of functions that encapsulate the focus logic, making it easier to add or remove focus strategies and improving the code's testability.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* Initialize focus in the popup:
*
* 1. If `toast` is `true`, don't steal focus from the document.
* 2. Else if there is an [autofocus] element, focus it.
* 3. Else if `focusConfirm` is `true` and confirm button is visible, focus it.
* 4. Else if `focusDeny` is `true` and deny button is visible, focus it.
* 5. Else if `focusCancel` is `true` and cancel button is visible, focus it.
* 6. Else focus the first focusable element in a popup (if any).
*
* @param {DomCache} domCache
* @param {SweetAlertOptions} innerParams
*/
const initFocus = (domCache, innerParams) => {
if (innerParams.toast) {
return
}
// TODO: this is dumb, remove `allowEnterKey` param in the next major version
if (!callIfFunction(innerParams.allowEnterKey)) {
blurActiveElement()
return
}
if (!focusButton(domCache, innerParams)) {
setFocus(-1, 1)
if (focusAutofocus(domCache)) {
return
}
if (focusButton(domCache, innerParams)) {
return
}
setFocus(-1, 1)
}
/**
* Initialize focus in the popup:
*
* 1. If `toast` is `true`, don't steal focus from the document.
* 2. Else if there is an [autofocus] element, focus it.
* 3. Else if `focusConfirm` is `true` and confirm button is visible, focus it.
* 4. Else if `focusDeny` is `true` and deny button is visible, focus it.
* 5. Else if `focusCancel` is `true` and cancel button is visible, focus it.
* 6. Else focus the first focusable element in a popup (if any).
*
* @param {DomCache} domCache
* @param {SweetAlertOptions} innerParams
*/
const initFocus = (domCache, innerParams) => {
if (innerParams.toast) return;
if (!callIfFunction(innerParams.allowEnterKey)) {
blurActiveElement();
return;
}
const focusHandlers = [
() => focusAutofocus(domCache),
() => focusButton(domCache, innerParams)
];
for (const handler of focusHandlers) {
if (handler()) return;
}
setFocus(-1, 1);
}


/**
* @param {DomCache} domCache
* @returns {boolean}
*/
const focusAutofocus = (domCache) => {
const autofocusElement = domCache.popup.querySelector('[autofocus]')
if (autofocusElement instanceof HTMLElement && dom.isVisible(autofocusElement)) {
autofocusElement.focus()
return true
}
return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper accessibility handling when focusing elements.

The focusAutofocus function correctly identifies and focuses an element with the [autofocus] attribute. However, consider verifying that this behavior aligns with accessibility standards, especially in scenarios where multiple autofocus elements might be present, which is generally discouraged but should still be handled gracefully.

}

/**
Expand Down