Skip to content

Commit aa27369

Browse files
authored
Dialog/Drawer: fix focus-restore scroll and dispose-while-open cleanup (#42544)
* Dialog/Drawer: don't scroll the page when restoring focus on close Pass { preventScroll: true } when returning focus to the trigger after close, so the page no longer jumps to the trigger (or to the top when scroll-padding-top is set). Fixes #38070, #41615, #35391. * Dialog/Drawer: restore body scroll when disposed while open dispose() now closes the native <dialog> and removes the dialog-open body class if the instance is torn down while still open (e.g. an SPA route change), instead of leaving overflow: hidden stuck on the body. Fixes #35934, #39910. * Bump bundlewatch size thresholds
1 parent c654bab commit aa27369

6 files changed

Lines changed: 61 additions & 6 deletions

File tree

.bundlewatch.config.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@
3434
},
3535
{
3636
"path": "./dist/js/bootstrap.bundle.js",
37-
"maxSize": "82.5 kB"
37+
"maxSize": "82.75 kB"
3838
},
3939
{
4040
"path": "./dist/js/bootstrap.bundle.min.js",
4141
"maxSize": "53.25 kB"
4242
},
4343
{
4444
"path": "./dist/js/bootstrap.js",
45-
"maxSize": "53.75 kB"
45+
"maxSize": "54.0 kB"
4646
},
4747
{
4848
"path": "./dist/js/bootstrap.min.js",

js/src/dialog-base.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,17 @@ class DialogBase extends BaseComponent {
117117
}, this._element, this._isAnimated())
118118
}
119119

120+
dispose() {
121+
// If disposed while still open, close the native <dialog> and restore body
122+
// scroll. Otherwise `dialog-open` (overflow: hidden) would stay stuck on the
123+
// body — e.g. when an SPA tears the component down mid-navigation.
124+
if (this._element.open) {
125+
this._closeAndCleanup()
126+
}
127+
128+
super.dispose()
129+
}
130+
120131
// Protected — hooks for subclasses to override
121132

122133
_getShowOptions() {

js/src/dialog.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
119119

120120
EventHandler.one(target, EVENT_HIDDEN, () => {
121121
if (isVisible(this)) {
122-
this.focus()
122+
this.focus({ preventScroll: true })
123123
}
124124
})
125125
})

js/src/drawer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
149149

150150
EventHandler.one(target, EVENT_HIDDEN, () => {
151151
if (isVisible(this)) {
152-
this.focus()
152+
this.focus({ preventScroll: true })
153153
}
154154
})
155155

js/tests/unit/dialog.spec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,28 @@ describe('Dialog', () => {
713713
expect(Dialog.getInstance(dialogEl)).toBeNull()
714714
expect(spyOff).toHaveBeenCalled()
715715
})
716+
717+
it('should close the dialog and restore body scroll when disposed while open', () => {
718+
return new Promise(resolve => {
719+
fixtureEl.innerHTML = '<dialog class="dialog" id="exampleDialog"></dialog>'
720+
721+
const dialogEl = fixtureEl.querySelector('.dialog')
722+
const dialog = new Dialog(dialogEl)
723+
724+
dialogEl.addEventListener('shown.bs.dialog', () => {
725+
expect(dialogEl.open).toBeTrue()
726+
expect(document.body.classList.contains('dialog-open')).toBeTrue()
727+
728+
dialog.dispose()
729+
730+
expect(dialogEl.open).toBeFalse()
731+
expect(document.body.classList.contains('dialog-open')).toBeFalse()
732+
resolve()
733+
})
734+
735+
dialog.show()
736+
})
737+
})
716738
})
717739

718740
describe('data-api', () => {
@@ -803,7 +825,7 @@ describe('Dialog', () => {
803825

804826
const hideListener = () => {
805827
setTimeout(() => {
806-
expect(spy).toHaveBeenCalled()
828+
expect(spy).toHaveBeenCalledWith({ preventScroll: true })
807829
resolve()
808830
}, 20)
809831
}

js/tests/unit/drawer.spec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,28 @@ describe('Drawer', () => {
570570
expect(Drawer.getInstance(drawerEl)).toBeNull()
571571
expect(spyOff).toHaveBeenCalled()
572572
})
573+
574+
it('should close the drawer and restore body scroll when disposed while open', () => {
575+
return new Promise(resolve => {
576+
fixtureEl.innerHTML = '<dialog class="drawer"></dialog>'
577+
578+
const drawerEl = fixtureEl.querySelector('dialog')
579+
const drawer = new Drawer(drawerEl)
580+
581+
drawerEl.addEventListener('shown.bs.drawer', () => {
582+
expect(drawerEl.open).toBeTrue()
583+
expect(document.body.classList.contains('dialog-open')).toBeTrue()
584+
585+
drawer.dispose()
586+
587+
expect(drawerEl.open).toBeFalse()
588+
expect(document.body.classList.contains('dialog-open')).toBeFalse()
589+
resolve()
590+
})
591+
592+
drawer.show()
593+
})
594+
})
573595
})
574596

575597
describe('data-api', () => {
@@ -649,7 +671,7 @@ describe('Drawer', () => {
649671
})
650672
drawerEl.addEventListener('hidden.bs.drawer', () => {
651673
setTimeout(() => {
652-
expect(spy).toHaveBeenCalled()
674+
expect(spy).toHaveBeenCalledWith({ preventScroll: true })
653675
resolve()
654676
}, 5)
655677
})

0 commit comments

Comments
 (0)