bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

offcanvas scrolls back to trigger element on close

Open lekoala opened this issue 2 years ago • 20 comments

Prerequisites

Describe the issue

When closing the offcanvas menu, the trigger element will get the focus due to

https://github.com/twbs/bootstrap/blob/cb021439c683d9805e2864c58095b92d405e9b11/js/src/offcanvas.js#L244-L249

this will make the whole page scroll back. If you scrolled (data-bs-scroll=true) or used an anchor, you will "jump back" to top of page.

maybe it may make sense in some situations, but at least there should be an option to disable that behaviour.

Reduced test cases

https://codepen.io/lekoalabe/pen/VwGYgdw

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

5.3.0-alpha1

lekoala avatar Feb 16 '23 13:02 lekoala

as a way around, you can just use the component directly without the data api, but that's still strange

lekoala avatar Feb 16 '23 13:02 lekoala

I think this should be the expected behavior if you use an anchor that takes you to a different page. However, if data-bs-scroll is set to true , why would we want to focus the trigger element in the first place since we are scrolling? Maybe we can add a condition for data-bs-scroll to be false on the this.focus() part

ChellyAhmed avatar Feb 16 '23 22:02 ChellyAhmed

@ChellyAhmed for one pagers, this wouldn't be the case, so offcanvas cannot assume that it should always focus back. also, it's inconsistent: why would the data api be doing that when using the toggle/hide feature wouldn't?

I agree with your proposal, I think a safe default would be: if you allow scrolling, then disable this.focus

lekoala avatar Feb 17 '23 08:02 lekoala

@julien-deramond Do you think it will be a good idea to create a pull request implementing the change mentioned?

ChellyAhmed avatar Feb 17 '23 08:02 ChellyAhmed

Just by reading quickly the discussion, difficult to say for sure if it is the right way to handle this issue. Maybe @GeoSot has some opinion about it already.

But IMO if you have an idea to improve the behavior, sure you can create a PR. I don't know if it is the appropriate change or not but having an example to play with in the PR would be helpful. I don't guarantee to be able to review it quickly but I'll try to do my best :) Thanks all for the improvement ideas.

julien-deramond avatar Feb 17 '23 09:02 julien-deramond

@julien-deramond you can have a look at the codepen. implementing the proposal would prevent jumping back to the top of the page when closing the menu if you clicked on section3 for example

lekoala avatar Feb 17 '23 11:02 lekoala

this will make the whole page scroll back. If you scrolled (data-bs-scroll=true) or used an anchor, you will "jump back" to top of page.

This may be a fine idea, but have in mind at that point, config is out of our scope, as we are outside Offcanvas's code

As Julien said, a Pr with the proper example, would help

GeoSot avatar Feb 17 '23 12:02 GeoSot

made a pr, and created another codepen with fixed sourcecode

https://codepen.io/lekoalabe/pen/RwYPQdW

vs not fixed

https://codepen.io/lekoalabe/pen/VwGYgdw

lekoala avatar Feb 17 '23 12:02 lekoala

I think @patrickhlauke may help there: I think focusing the trigger back is mandatory for keyboard users.

ffoodd avatar Feb 17 '23 12:02 ffoodd

I think focusing the trigger back is mandatory for keyboard users

yes. unless you want to offer a way for authors to suppress this behavior (for regular modals as well) if they for whatever reason want to do their own post-close focus handling, and we stress this in the documentation

patrickhlauke avatar Feb 17 '23 14:02 patrickhlauke

ok so not restoring focus is not an option, but then the question is more about what needs to be focused. if the anchor has changed, it makes no sense to focus back to the original button. maybe we can simply set the focus to the new anchor then?

lekoala avatar Feb 17 '23 14:02 lekoala

I am wondering, why don't we just restore focus simply without scrolling? We can do that simply by adding preventScroll: true to the focus method. I opened a pull request above to demonstrate that.

ChellyAhmed avatar Feb 17 '23 14:02 ChellyAhmed

@ChellyAhmed your solution is better than mine maybe even tweak it a little and set it to

  this.focus({
    preventScroll: this._config.scroll
  })

showed here https://codepen.io/lekoalabe/pen/OJoVwyp

lekoala avatar Feb 17 '23 14:02 lekoala

Keyboard users wouldn't know focus was restored without scroll. There's a reason for this standard behaviour.

You're talking about anchor change, but opening offcanvas shouldn't change anchor. Those are two separate actions.

ffoodd avatar Feb 17 '23 15:02 ffoodd

as said, I'd offer the option maybe (both for "don't actually focus anything, the author will do their own focusing" and for "yeah, set focus back, but don't visibly scroll" - though that would be quite confusing, and a keyboard user would then need to Tab to the next focusable element for the viewport to snap back to where they actually are), but very strongly stress the implications of these in the docs

patrickhlauke avatar Feb 17 '23 15:02 patrickhlauke

@lekoala Thanks for your comment. But I am not entirely sure we should add the : this._config.scroll. Because if the user did not scroll, we wouldn't care anymore about whether we should scroll back or not, because the user did not scroll in the first place. Also, following up on @ffoodd, when a keyboard user opens the offcanvas menu, it would be through the trigger element. Hence, IMO it would make sense that this trigger element is still focused regardless of the scrolling position.

ChellyAhmed avatar Feb 17 '23 15:02 ChellyAhmed

assuming we disable the automatic behaviour, IMO, to allow the user to choose to scroll back to the initial position, I think it depends on the placement of the trigger element, if we decide to place it initially on a sticky navbar, it will follow the navigation through the page, and when the user wants to go back to the initial starting point, he would only have to use the trigger element once again since it follows him.

On another hand, we can just make the trigger element follow you while navigating, for example, we can make it stick to the top once you use it to scroll down.

opinion?

ruiarii avatar May 13 '23 18:05 ruiarii

i am having the same issue i need help

Maria-qtr2 avatar Aug 06 '23 08:08 Maria-qtr2

As a temporary solution u can ommit keyboard users, and use link without href instead of button for burger, this works as ur expect it to work, our use ur burger button inside fixed-top menu.

 <a class="navbar-toggler" data-bs-toggle="offcanvas" data-bs-target="#offcanvasNavbar"
                aria-controls="offcanvasNavbar" aria-label="burger">
                <span class="navbar-toggler-icon"></span>
            </a>

^^ this works fine

i am having the same issue i need help

Prerequisites

Describe the issue

When closing the offcanvas menu, the trigger element will get the focus due to

https://github.com/twbs/bootstrap/blob/cb021439c683d9805e2864c58095b92d405e9b11/js/src/offcanvas.js#L244-L249

this will make the whole page scroll back. If you scrolled (data-bs-scroll=true) or used an anchor, you will "jump back" to top of page.

maybe it may make sense in some situations, but at least there should be an option to disable that behaviour.

Reduced test cases

https://codepen.io/lekoalabe/pen/VwGYgdw

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

5.3.0-alpha1

Extazykeep avatar Jun 10 '24 09:06 Extazykeep

I think we should add an extra option like scrollBackAfterClose (default = true).
Then we have the ability to change the behavior like we need.

Xcreen avatar Jun 12 '24 12:06 Xcreen