Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Autocomplete] List gets scrolled to the top when new data is received #29508

Closed
2 tasks done
SuperChuck83 opened this issue Nov 4, 2021 · 33 comments · Fixed by #35735
Closed
2 tasks done

[Autocomplete] List gets scrolled to the top when new data is received #29508

SuperChuck83 opened this issue Nov 4, 2021 · 33 comments · Fixed by #35735
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! regression A bug, but worse v5.x migration

Comments

@SuperChuck83
Copy link

SuperChuck83 commented Nov 4, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When you load data from server by scrolling in autocomplete, then adding this data in the "options" state, the scroll is going to the up of the option pop up

Expected Behavior 🤔

in Material UI v4 scroll stay at his position, you can scrolldown to see new data

Steps to Reproduce 🕹

Steps:

  1. autocomplete with method for detecting scroll bottom
  2. when you scroll at bottom add X elements
  3. scroll go to the up

Context 🔦

Your Environment 🌎

@emotion/react: ^11.5.0 => 11.5.0
@emotion/styled: ^11.3.0 => 11.3.0
@mui/core: 5.0.0-alpha.53
@mui/icons-material: ^5.0.5 => 5.0.5
@mui/lab: ^5.0.0-alpha.53 => 5.0.0-alpha.53
@mui/material: ^5.0.6 => 5.0.6
@mui/private-theming: 5.0.1
@mui/styled-engine: 5.0.2
@mui/styles: ^5.0.2 => 5.0.2
@mui/system: 5.0.6
@mui/types: 7.0.0
@mui/utils: 5.0.1
@types/react: ^16.9.52 => 16.9.52
react: ^16.14.0 => 16.14.0
react-dom: ^16.14.0 => 16.14.0
typescript: ^3.9.10 => 3.9.10

@SuperChuck83 SuperChuck83 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 4, 2021
@mnajdova
Copy link
Member

mnajdova commented Nov 5, 2021

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

@mnajdova mnajdova added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 5, 2021
@rbjunior000
Copy link

rbjunior000 commented Nov 17, 2021

I have the same issue, minimal code repro is here: https://codesandbox.io/s/zen-tdd-sgbeh?file=/src/Demo.tsx

@rbjunior000

This comment was marked as outdated.

@SuperChuck83
Copy link
Author

SuperChuck83 commented Nov 17, 2021

Hello @rbjunior000,
thx for the codeSandbox ! i tried to help you a few on the code but i did not succeed.

i did it on my project by this method :

  1. we use onHighlightChange={onHighlightChange} for testing scroll

  2. const onHighlightChange = function (event: React.ChangeEvent<{}>, option: T | null, reason: AutocompleteHighlightChangeReason) {
    var length = options.length;

    var indexOption = options.indexOf(option);

       //On lance la recherche si on survole un des 5 derniers éléments
       if (indexOption >= length - 5) {
           let targetElem = event.target as any;
           (async () => {
              
               **await getDatas 
               if (targetElem && typeof targetElem.scrollIntoView === 'function') targetElem.scrollIntoView();**
           })();
       }
    
}

@marr
Copy link

marr commented Nov 30, 2021

Here is a simple repro case: https://codesandbox.io/s/autocomplete-scroll-bug-39wfv
It implements the custom onScroll solution suggested from @oliviertassinari here: #18450 (comment)

@thefenry
Copy link

This is an issue for us as well. Had the original solution as stated in the previous comment but now that we upgraded the paging does not work as it did. We not get set back to the top of the list.

@marr

This comment was marked as off-topic.

@thefenry

This comment was marked as resolved.

@julian-pineiro

This comment was marked as resolved.

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Apr 11, 2022
@SuperChuck83
Copy link
Author

SuperChuck83 commented Apr 12, 2022

Hello every one i resolved my issue by doing a scroll, like i said before i use prop "onHighlightChange" from "Autocomplete" to detect if i need to get my data from server or not.

` const onHighlightChange = function (event: React.ChangeEvent<{}>, option: T | null, reason: AutocompleteHighlightChangeReason) {

    var length = options.length;
    if (!pagination || option === null || loading || disableSearchByScroll || length < 5) {
        return;
    }
    var indexOption = options.indexOf(option);
      
        //On lance la recherche si on survole un des 5 derniers éléments
        if (indexOption >= length - 5) {
            let targetElem = event.target as any;
            (async () => {
                setLoading(true);
                await getDatasCore(true);
                if (targetElem && typeof targetElem.scrollIntoView === 'function') targetElem.scrollIntoView();
            })();
        }
}`

event: React.ChangeEvent<{}> => let targetElem = event.target as any; => if (targetElem && typeof targetElem.scrollIntoView === 'function') targetElem.scrollIntoView();

in this way when data are added to the component i scroll at bottom ...

@BiancaArtola

This comment was marked as off-topic.

@julian-pineiro
Copy link

julian-pineiro commented Apr 20, 2022

Hey @SuperChuck83, that works alright, but still has the bug mentioned in #18450 (comment) so the select goes back to default state (scrolls to the top). Any idea on how to address this issue? I'm currently trying to adapt this code that seems to work: https://codesandbox.io/s/material-demo-0fbyb?file=/ExampleWrapper.js

@mnajdova mnajdova assigned michaldudak and unassigned hbjORbj Aug 31, 2022
@mnajdova mnajdova added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 31, 2022
@michaldudak michaldudak changed the title Migration V4 -> V5 autocomplete issue [Autocomplete] List gets scrolled to the top when new data is received Oct 28, 2022
@michaldudak
Copy link
Member

We'll gladly accept community contributions to fix this problem.

@michaldudak michaldudak removed their assignment Oct 28, 2022
@sai6855
Copy link
Contributor

sai6855 commented Jan 10, 2023

We'll gladly accept community contributions to fix this problem.

@michaldudak created pr for the issue. #35735 and working demo: https://codesandbox.io/s/nervous-euclid-2ekej7

@ZeeshanTamboli
Copy link
Member

To the community,

It would really help us if the fix in #35735 was tested within your applications. You can replace the @mui/material version in your package.json with the following link:

"@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/fb77c56f/@mui/material"

@carvillx
Copy link

To the community,

It would really help us if the fix in #35735 was tested within your applications. You > can replace the @mui/material version in your package.json with the following link:
.json with the following link:

"@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/fb77c56f/@mui/material"

Can confirm this is working great ☝️

Just to note, if you are looking at using a custom component (e.g Popper, Paper, Listbox) you will need to wrap that in a useMemo like the following:

    const customPaper = useMemo(
        () => (paperProps: any) => {
            return (
                <Dropdown ref={paperProps.dropdownRef} elevation={0} data-prevent-dismiss>
                    {paperProps.children}
                </Dropdown>
            );
        },
        [],
    );

and set the prop of PaperComponent to the new custom component on the Autocomplete like the following:

    <Autocomplete
       .....
       PaperComponent={customPaper}
     />

Excellent work everyone! When is this likely to get merged?

@oliviertassinari
Copy link
Member

in Material UI v4 scroll stay at his position, you can scrolldown to see new data

Could we do a bisection and find which commit introduced the regression? The logic around the scroll sync is one of the hardest of the component to get right. I think that it's important we get the feedback on why this broke.

@michaldudak
Copy link
Member

This is the PR that introduced the issue: #23718
It seems it was done on purpose, as even the new test was added to verify if the highlight resets when options are added. @oliviertassinari, you seem to be the author of this test. Do you remember more context about this change?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 29, 2023

@michaldudak Ah ok, this is the issue we use for https://github.com/mui/react-technical-challenge, I linked it again (as I transferred the GitHub issue to a private repo). We want to sync the highlight with the props but we don't necessarily care about resetting the scroll.

@Dustin-Digitar
Copy link
Contributor

Did the fix actually get merged?

@sai6855
Copy link
Contributor

sai6855 commented Jun 19, 2023

Did the fix actually get merged?

Yes, fix is merged. If you are facing this can you please provide repro.

@dbehmoaras
Copy link

If anyone is still having issues, try this approach instead:

To solve this in our codebase, I followed the approach in the youtube link below instead, using the MUI text field for the input and the MUI popper for the dropdown feature, effectively rebuilding the Autocomplete using two separate components. In our case, I used the IntersectionObserver to fire fetchMore query when reaching the bottom element (customizable to your desire of course) and used the ResizeObserver to make it responsive to viewport size. With this approach, it helps to think of infinite scroll as a paginated query, increasing the offset every time you reach the bottom of the list. You could get fancy as well by adding a loader each time the fetchMore query fires.
https://www.youtube.com/watch?v=NZKUirTtxcg

Full credit goes to Web Dev Simplified for sharing this approach. (Before anyone asks, no I am not Web Dev Simplified nor am I involved with the channel or its members in anyway).

@tihuan
Copy link

tihuan commented Nov 15, 2023

Did the fix actually get merged?

Yes, fix is merged. If you are facing this can you please provide repro.

Sorry I might be blind, which release includes this fix now? Thank you!

@oliviertassinari
Copy link
Member

This bug fix was released in v5.11.7: https://github.com/mui/material-ui/releases/tag/v5.11.7.

@tihuan
Copy link

tihuan commented Nov 20, 2023

Got it 👍 Thanks so much, @oliviertassinari 🙏 !

@hienhhcc
Copy link

The problem still persist if we add PaperComponent prop

@ZeeshanTamboli
Copy link
Member

The problem still persist if we add PaperComponent prop

@hienhhcc See issue #31073 which shows a solution as well.

@aneeshikmat
Copy link

If anyone still has the issue, just make sure you are not set a dynamic value / props inside PaperComponent direct, you need to use slotProps instead of using dynamic value direct, check this example from real project:

Fix issue:

  const customPaper = useMemo(
    () => (paperProps) => {
      const isReachedMax = paperProps.selectedValues.length > paperProps.maxAllowedItems;
      const text = !isReachedMax ? "..." : ".....";

      return (
        <Paper className={paperProps.className}>
          {paperProps.children}

          {paperProps.showDisclaimer && (
            <DisclaimerLabel
              text={text}
              sx={{
                color: isReachedMax ? "red" : "inherit",
              }}
            />
          )}
        </Paper>
      );
    },
    []
  );
slotProps={{
        paper: {
          selectedValues: values,
          maxAllowedItems: props.maxAllowedItems,
          showDisclaimer: props.showDisclaimer,
        },
      }}

@shashank-campx
Copy link

@aneeshikmat What about the type override of PaperProps ?

@emike108
Copy link

@aneeshikmat What about the type override of PaperProps ?

Yes I have the same question, slotProps helped A LOT but now getting some type issues

@aneeshikmat
Copy link

aneeshikmat commented Jun 15, 2024

@aneeshikmat What about the type override of PaperProps ?

@aneeshikmat What about the type override of PaperProps ?

Yes I have the same question, slotProps helped A LOT but now getting some type issues

maybe you need to be set as a Partial<PaperProps> I'm not sure about the kind of error...

slotProps={{
        paper: {
....
        } as Partial<PaperProps>
      }}

@sai6855
Copy link
Contributor

sai6855 commented Nov 6, 2024

@MathewThekk can you provide a reproduction, instead of raw code? it will be easier to debug

@MathewThekk
Copy link

@sai6855 Hey, i found the issue to be on my end. My component was being unmounted and remounted instead of rerendering. This was caused by wrong handling by the parent component. I deleted my comment to not confuse anyway. Thanks for response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! regression A bug, but worse v5.x migration
Projects
None yet