For accessibility reasons (for people only using keyboard to navigate), the popover should have the possibility to be closed by pressing ESC.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

sylvainar created an issue. See original summary.

sylvainar’s picture

Issue summary: View changes
FileSize
491 bytes
sylvainar’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Status: Active » Needs review
sylvainar’s picture

FileSize
491 bytes

Fixed a typo on the second patch.

sylvainar’s picture

FileSize
533 bytes

When you press ESC everywhere in the site if there is no popup, you got an error in the console. I modified the patch.

sylvainar’s picture

FileSize
533 bytes

Fix typo

sylvainar’s picture

FileSize
548 bytes

Fix typo

markcarver’s picture

Title: Accessibility : Close popover when ESC is pressed » Accessibility: Close popover when ESC is pressed
Status: Needs review » Needs work
Issue tags: +needs backport to 7.x-3.x

I'm all for adding accessibility, however this feature further deviates from the native Bootstrap plugin (more so than it already has).

To keep BC, I would say this would also need a setting that could be disabled if such functionality is not desired, for whatever reason.

---

  1. +++ b/js/popover.js
    @@ -60,6 +60,15 @@ var Drupal = Drupal || {};
    +          .keyup(function (event) {
    

    This should be .on('keyup.bs.popover', function (e) { instead.

  2. +++ b/js/popover.js
    @@ -60,6 +60,15 @@ var Drupal = Drupal || {};
    +            // Close popup when ESC is pressed.
    

    Comment should be above the method/anonymous function invocation with a space before it as well (in between chain).

  3. +++ b/js/popover.js
    @@ -60,6 +60,15 @@ var Drupal = Drupal || {};
    +            if (event.which === 27) {
    +              if($currentPopover) {
    

    Also, there's no need for two if code blocks here, just use one with &&.

markcarver’s picture

bump...