Add keydown listener to backdrop to enable static offcanvas to close on Esc

[Ronid1]

Description

Added keydown event listener to backdrop.
Updated static offcanvas to close on esc when data-bs-keyboard = true, which is also the current default setting for this attribute.
If data-bs-keyboard = false the offcanvas will not close on any esc press.

Motivation & Context

This is a bug fix and provides offcanvas with similar behavior on esc keydown to modal, esc pressed on either static offcanvas or its backdrop will close it.

Type of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Refactoring (non-breaking change)
• Breaking change (fix or feature that would change existing functionality)

Checklist

• I have read the contributing guidelines
• My code follows the code style of the project (using npm run lint)
• My change introduces changes to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

Live previews

Change can be verified using this code snippet (showcases all scenarios):

<!--Static offcanvs with data-bs-keyboard=true closes on esc-->
    <button class="btn btn-primary" type="button" data-bs-toggle="offcanvas" data-bs-target="#staticBackdrop"
        aria-controls="staticBackdrop">
        static offcanvas closes on esc
    </button>
    <div class="offcanvas offcanvas-start" data-bs-backdrop="static" data-bs-keyboard="true" tabindex="-1"
        id="staticBackdrop" aria-labelledby="staticBackdropLabel">
        <div class="offcanvas-header">
            <h5 class="offcanvas-title" id="staticBackdropLabel">Offcanvas</h5>
            <button type="button" class="btn-close" data-bs-dismiss="offcanvas" aria-label="Close"></button>
        </div>
        <div class="offcanvas-body">
            <div>
                I will not close if you click outside of me, but will close on esc
            </div>
        </div>
    </div>

    <!--Static offcanvs with data-bs-keyboard=false attribute dosen't close on esc-->
    <button class="btn btn-primary" type="button" data-bs-toggle="offcanvas" data-bs-keyboard="false"
        data-bs-target="#staticBackdropNoKeyboard" aria-controls="staticBackdrop">
        static offcanvas closes on esc
    </button>
    <div class="offcanvas offcanvas-start" data-bs-backdrop="static" tabindex="-1" id="staticBackdropNoKeyboard"
        aria-labelledby="staticBackdropLabel">
        <div class="offcanvas-header">
            <h5 class="offcanvas-title" id="staticBackdropLabel">Offcanvas</h5>
            <button type="button" class="btn-close" data-bs-dismiss="offcanvas" aria-label="Close"></button>
        </div>
        <div class="offcanvas-body">
            <div>I will not close if you click on press esc outside of me.</div>
        </div>
    </div>

    <!--Non-static offcanvs behavior not changed-->
    <button class="btn btn-primary" type="button" data-bs-toggle="offcanvas" data-bs-target="#nonStaticBackdrop"
        aria-controls="staticBackdrop">
        non-static offcanvas
    </button>
    <div class="offcanvas offcanvas-start" tabindex="-1" id="nonStaticBackdrop" aria-labelledby="staticBackdropLabel">
        <div class="offcanvas-header">
            <h5 class="offcanvas-title" id="staticBackdropLabel">Offcanvas</h5>
            <button type="button" class="btn-close" data-bs-dismiss="offcanvas" aria-label="Close"></button>
        </div>
        <div class="offcanvas-body">
            <div>I will close if you click outside of me.</div>
        </div>
    </div>

Related issues

Closes #37155

[GeoSot]

Can you explain me, why you set the listener over the parent element?

[Ronid1]

Can you explain me, why you set the listener over the parent element?

The backdrop, being a part of another element, is always out of focus (e.g. the focus is on the modal/offcanvas element itself), and therefore a keydown event will not be triggered on it.
I double-checked my code and I agree that using the parent element isn't the best approach, since it doesn't work if the offcanvas is placed in another div.
According to the documentation only one offcanvas/ modal can be opened at once, and these are the only two elements using the backdrop so I think a safe solution is setting the event listener on the document element.

[GeoSot]

this being said, I am not sure if we need to change something on offcanvas.

The initial point is to enable esc behavior in case of static backdrop. Another approach that seems fine too, is focusTrap to trap clicks too, in order not to let user get focus from the trapped element. Esc would work in that case too

/cc @patrickhlauke @julien-deramond

[Ronid1]

The initial point is to enable esc behavior in case of static backdrop. Another approach that seems fine too, is focusTrap to trap clicks too, in order not to let user get focus from the trapped element. Esc would work in that case too

I've explored the second approach as well, and it is also a possibility. Personally, I think this should remain a backdrop functionality for now, since this discussion is about the wanted behavior of the backdrop and not of any element in focus. There might be a place in the future for extending the focustrap but I'm not sure it is currently needed.

[GeoSot]

@patrickhlauke @twbs/js-review your opinion would help here

[GeoSot]

Hey @Ronid1 . Thanks for the effort here.
I checked some things, but in the end you will need to add the proper tests too

[Ronid1]

@GeoSot I made changes according to your review and added tests. The pipeline is clear now.

[louismaximepiton]

The changes look great to me !

I don't know if it is wanted since it's not the case for modals but clicking on the static backdrop sets the focus on the close button. It leads to a display of a box-shadow. Feels a bit weird using this but why not.

It could be tackled in another PR but my only question here is: Should we use this new functionality in the modals as well ? Because it seems like the backdrop escape event is handled differently in the 2 components.

[Ronid1]

Made changes requested by @louismaximepiton. Bundlewatch seems to be failing due to max size. Can you please help? @GeoSot

[GeoSot]

I have it in my mind, and I apologize for the delay. Team, at this point, has as priority the major css changes.
So I have postponed it, in order to discuss it later. And of course, we will handle the bundlewatch then 😄

Many many thanks for your patience

I was made by accident