material-ui
material-ui copied to clipboard
[Tabs] Fix and improve visibility of scrollButtons using IntersectionObserver
Fixes the issues below: #31936 #19673 #33349 #23711
This fix handle visibility of scroll buttons specially in dynamic content behavior and also it reduces the complexity of the code (calculation logic).
before: https://codesandbox.io/s/interesting-before-b3kdvn?file=/src/App.js after: https://codesandbox.io/s/interesting-after-zej36n?file=/src/App.js
- [x] I have followed (at least) the PR section of the contributing guide.
Netlify deploy preview
https://deploy-preview-36071--material-ui.netlify.app/
Bundle size report
Details of bundle changes (Toolpad) Details of bundle changes
Generated by :no_entry_sign: dangerJS against ca7b7b287498071b10268f96c3bd522a6aa8412f
I've implemented a new method to handle the visibility of scroll buttons, using the IntersectionObserver. Essentially, it allows us to detect when a particular element (in this case, a tab) is in view on the screen.
With this new method, we're observing only the first and last tabs. If either the first or last tab is partially visible or completely invisible, we'll show the appropriate scroll button. For example, if the first tab is only partially visible,we'll show the scroll button that allows the user to scroll to the left and reveal more of the first tab. If the last tab is completely invisible, we'll show the scroll button that allows the user to scroll to the right and reveal the last tab.
By using the IntersectionObserver to only observe the first and last tabs, we're optimizing our code and reducing unnecessary calculations especially in dynamic content scenarios. This new method, the code should be easier to read and maintain.
Hi @mnajdova if you have a moment, could you kindly take a look and review this PR please.
@mnajdova @SaidMarar Are there any updates on this issue?
@mnajdova @SaidMarar Are there any updates on this issue?
@JoinerDev the PR needs only to be reviewed, so we can advance.
Hello @ZeeshanTamboli , Thank you a lot for your review :smile:
-
For the first issue you mentioned in the demos, i don't reproduce it with the latest changes, i have the same behavior as the one in prod. Please check the screenrecords below:
-
Dev:
-
Prod:
-
-
For the second issue in Argos, i just fixed it by removing debounce on both observers, because it causes a delay on render for the buttons.
PR is ready, @ZeeshanTamboli review is welcome :smiley:
For the first issue you mentioned in the demos, i don't reproduce it with the latest changes, i have the same behavior as the one in prod.
I can still see it. Can you tell me your operating system, browser, and its version? I'm using Windows 10 and Chrome Version 114.0.5735.199 (64-bit). I am also seeing it on Firefox.
@ZeeshanTamboli Could you please post your resolutions, my environment test on Ubuntu 22.04:
- Firefox 115.0 (64 bits), resolution 1920px 995px
- Chrome Version 114.0.5735.198 (Build officiel) (64 bits), resolution 1920px 995px
My screen resolution is 1920px 1080px.
@ZeeshanTamboli it works also on iPhone.
https://github.com/mui/material-ui/assets/20561190/cecbef2f-0000-4b3e-b9e4-41c9b1efb869
@ZeeshanTamboli can you screenrecord,maybe it will help me to reproduce.
On Windows 10, Chrome Browser:
https://github.com/mui/material-ui/assets/20900032/543e8be9-e7cb-4352-a1b3-639b9baf970a
On Android Chrome:
https://github.com/mui/material-ui/assets/20900032/eed7564f-57cb-4c5d-aa6b-c264631e49a0
@ZeeshanTamboli When you reach the last element could you please print the getBoundingClientRect (value of the attribute right) of:
- The last element in tabs the button (item seven)
- The element with class MuiTabs-scroller
@ZeeshanTamboli When you reach the last element could you please print the getBoundingClientRect (value of the attribute right) of:
- The last element in tabs the button (item seven)
- The element with class MuiTabs-scroller
@SaidMarar
https://github.com/mui/material-ui/assets/20900032/c48b1797-a020-4dd5-8099-864a4ee6dd3d
Thank you, @ZeeshanTamboli. It was very helpful, and I was able to reproduce it in both Chrome and Firefox (on windows) and it happens only in 1920 resolution. The problem persists even when we reach the maximum value of scrollLeft (the max value: scrollWidth - clientWidth). The last item on the x-axis always remains higher than its parent.
And this is a known issue and it was reported on chromium too, as you can see in the example the scrollLeft is returning a decimal value which is a bug.
For firefox i have doubts they are rounding the scrollLeft but the issue of boundingRect stays the same as chromium.
Possible solution is to use 0.99 as threshold, i tested it on local it resolved the problem.
@ZeeshanTamboli could you please test again ? :)
The latest updates on your projects. Learn more about Argos notifications ↗︎
Waiting for the first build to start…
@ZeeshanTamboli if updateScrollButtons
will remain accessible for developers who are relying on it, why we dont just change its old implementation to use IntersectionObserverAPI. The code will be simpler.
we create a state that is responsible to re-init the observer:
const [updateScrollObserver, setUpdateScrollObserver] = React.useState(false);
we change the updateScrollButtonState implementation to this:
const updateScrollButtonState = useEventCallback(() =>
setUpdateScrollObserver(!updateScrollObserver),
);
Finally we just append the Observer useEffect dependencies:
[scrollable, scrollButtons, updateScrollObserver, childrenProp?.length];
What do you think ?
I think I understand what you mean. Whenever the updateScrollButtons
action is triggered by the developer, you want to run the IntersectionObserver i.e the useEffect? Sounds like a good idea. Feel free to apply the changes. But when will you reset this scroll observer state?
@ZeeshanTamboli Yes the IntersectionObserver effect will run, when the developer triggers updateScrollButtons
action.
Sorry, I don't understand what you mean by when i will reset the scroll observer state ?
Sorry, I don't understand what you mean by when i will reset the scroll observer state ?
Does the updateScrollObserver
state, triggered by updateScrollButtons
action, reset to false after the effect finishes running?
Does the
updateScrollObserver
state, triggered byupdateScrollButtons
action, reset to false after the effect finishes running?
No need to reset, because the updateScrollObserver
state is used only to be toggled every time we call updateScrollButtons
, ensuring the re-run of the effect, which means rebinding the IntersectionObserver to detect the visibility of buttons.
@SaidMarar It looks good! Thanks for your great contribution!
Thanks to you too for your assistance and help 👍🏻