react
react copied to clipboard
Bug: Disabled button state update prevents `scrollIntoView`
I have a button that triggers scrollIntoView for a node when clicked. It also updates some state which subsequently disables that button in certain cases. If the button becomes disabled, the scrollIntoView execution aborts.
This seems to only happen in Chrome/Brave.
React version: 17.0.1
Steps To Reproduce
See steps to reproduce in sandbox.
https://codesandbox.io/s/goofy-curie-yts68?file=/src/App.js
The current behavior
Described above.
The expected behavior
The state update should not prevent scrollIntoView.
hi @jjenzz can you try following solution. may it will help.
setTimeout(() => {
setActiveItem(1);
}, 1000);
Reproduces with:
- 16.14.0
experimentalandcreateRootandcreateBlockingRoot
Does not reproduce when:
- using
onMouseDowninstead ofonClick
Experimented a bit with the suggestion of @arvind-nikam-halodoc and it stops scrolling once the state update is triggered. For example, with
setTimeout(() => {
setActiveItem(1);
}, 300);
it starts scrolling and stops right in the middle once the timer runs.
More confusion:
Calling scrollIntoView from a button that isn't being disabled works: https://codesandbox.io/s/onclick-scrollintoview-cancellation-workaround-z7si1
This is not a scrollIntoView issue, but rather a scrollIntoView({ behavior: "smooth" }) issue. Changing "smooth" to "instant" fixes behavior. So I assume it has to do something with the browser having to maintaining focus when it is a smooth scroll and react's re-render causes us to lose that focus for a temporary second. Unfortunately I could not find any implementation details of scrollIntoView so this is just a guess.
A hack you can do is to trigger the setState and then call the scroll in a setTimeout to throw the scroll into the eventloop letting the render happen first such as:
setActiveItem(1);
setTimeout(() => {
//using a ref here
slideNode.current.scrollIntoView({block:"end", behavior: "smooth" });
}, 0);
So I assume it has to do something with the browser having to maintaining focus when it is a smooth scroll
Interestingly, this doesn't happen in other browsers as far as I could tell. Perhaps I should raise a Chrome issue?
A hack you can do
Indeed. This was the first thing I had tried but not particularly keen on littering setTimeout if it can be avoided. I may just go with it for now though as it does feel strange to use DOM API for this case.
Thanks for the help ☺️
@jjenzz Please try this:
const slideNode = document.getElementById(`item-${1}`);
setActiveItem(1);
requestAnimationFrame(() => slideNode.scrollIntoView({ behavior: "smooth" }));
https://codesandbox.io/s/nice-agnesi-q4g0v
And also I think this issue is about re-rendering
@behnammodi Yes, I am aware that delaying things will make it work, these are all things I tried when debugging it but I shouldn't have to do that. It works perfectly fine in other browsers.
Thanks for the help though 🙂
Hi there,
React will refocus an element after it applied effect mutation. However, since your button is disabled, React can not perform refocus. Actually, it has tried to refocus but unsuccessfully. The process of refocusing blocks the scroll run smoothly. And sure, this happened with all elements having disabled attribute. React also acknowledges (comments on the code of restoreSelection) that by refocusing, it's undesirable that focusing a node can change the scroll position.
If you might wonder why it's related to focus, clicking a button triggers focus of that element. React performed mutation that has deletion and had to refocus the priorFocusedElem later. But this time, it failed.
In fact, it's not necessary to refocus an element that have disabled attribute so we should check if it's a disabled element or not. I've created a pull request at #21480. Please have a look.
I had the same issue and found a workaround for this. Basically I just added the scroll into view call inside a setTimeout, effectively waiting for all the react state updates before scrolling.
setTimeout(() => {
ref.current?.scrollIntoView({ behavior: "smooth" });
}, 0);
I had a similar issue without even any disabled button state and "fixed" it with requestAnimationFrame.
- react 17.0.2
Before:
useEffect(() => {
if (scrollToTodayRequired && todayRef.current) {
todayRef.current.scrollIntoView({
behavior: "smooth",
});
setScrollToTodayRequired(false); // <= this line will cancel the scrollIntoView
}
}, [scrollToTodayRequired, todayRef.current]);
If you remove behavior (instant) then it's working.
After:
useEffect(() => {
if (scrollToTodayRequired && todayRef.current) {
requestAnimationFrame(() => {
if (todayRef.current) {
todayRef.current.scrollIntoView({
behavior: "smooth",
});
}
});
setScrollToTodayRequired(false);
}
}, [scrollToTodayRequired, todayRef.current]);
Now, this is working as expected. Somehow the smooth scrollIntoView was being canceled by the setState which I have a hard time understanding.
Hi,
After going through your sandbox and trying what everyone else has suggested, It seems that this bug seems to be more related to the browser's scrollIntoView implementation.
However, this seems to work without making too many changes to your code:
instead of using scrollIntoView, I called the container node's scrollTo function and passed it the offsetLeft of the element you wanted the button to scroll to.
Sandbox with the alternate solution
hope this helps !
P.S. Only tried this is Chrome
We also ran into the original issue. Wrapping the smooth scrollTo in setTimeout works.
https://github.com/facebook/react/pull/22256 would probably be a fine fix for this issue.
On macOS it only manifests in Chrome, not in Safari.
A simple fix (and one that doesn't involve setTimeout) would be to call .blur() on the button before .scrollIntoView() is triggered. E.g.
onClick={(event) => {
event.target.blur();
const slideNode = document.getElementById(`item-${1}`);
slideNode.scrollIntoView({ behavior: "smooth" });
setActiveItem(1);
}}
This way the button loses focus right before the scroll occurs, which means Chrome won't detect a change mid-scroll.
Should note that it's probably best for UX/accessibility to only blur when necessary. So probably a good idea to wrap the .blur() inside an if that checks to see if the button is going to be disabled (e.g. if (index === length - 2) or something like that). For example: https://stackoverflow.com/a/78409295/4157821
In my case only setting buttonElement.disabled = true right after #.scrollTo({ behavior: "smooth", top: 0 }) fixes it