chatscope / chat-ui-kit-react

Build your own chat UI with React components in few minutes. Chat UI Kit from chatscope is an open source UI toolkit for developing web chat applications.
https://chatscope.io
MIT License
1.34k stars 116 forks source link

1px fix for Firefox not necessarily working #42

Open nzayatz14 opened 3 years ago

nzayatz14 commented 3 years ago

Hi there,

I began testing scroll behavior for the component on Firefox and I noticed it was doing this strange thing when I would scroll to the end of my messages. It would let me keep scrolling past the end of my messages, but at a very slow pace:

ezgif com-gif-maker (1)

I dug into the MessageList component to see if I could find anything that would cause this odd behavior.

I did notice there was a few blocks that mentioned a 1px fix for Firefox and thought maybe that had something to do with it.

// 1 px fix for firefox
    const sticky =
      list.scrollHeight === topHeight ||
      list.scrollHeight + 1 === topHeight ||
      list.scrollHeight - 1 === topHeight;

and

// If was sticky because scrollHeight is not changing, so here will be equal to lastHeight plus current scrollTop
          // 1px fix id for firefox
          const sHeight = list.scrollTop + this.lastClientHeight;
          if (
            list.scrollHeight === sHeight ||
            list.scrollHeight + 1 === sHeight ||
            list.scrollHeight - 1 === sHeight
          ) {
            if (autoScrollToBottom === true) {
              this.scrollToEnd(this.props.scrollBehavior);
              this.preventScrollTop = true;
            }
          } else {
            this.preventScrollTop = false;
          }

I decided to set a breakpoint at one of these positions and noticed that, on Firefox, list.scrollHeight is not consistent at all. Every hover over list.scrollHeight resulted in a different height, some as far as 4px off from the topHeight. See my gif below:

ezgif com-gif-maker

Now, I'm not 100% sure what these "1px checks" are doing, but I think its safe to say that even a 1px margin of error is not enough. Ideally there is a way to not rely on these "1px checks." It may also help to specifically check if the scroll position has gone past either end of the content and just reset it then.

Thanks!

nzayatz14 commented 3 years ago

UPDATE: I was able to duplicate this behavior in this Storybook Example by setting the font-size on the body tag to 0.875rem

Screen Shot 2021-05-07 at 5 32 51 PM

ezgif com-gif-maker

supersnager commented 3 years ago

Hi @nzayatz14, thank you for the great research. I use Perfect scroll for scrolling and this issue has a long story here: https://github.com/mdbootstrap/perfect-scrollbar/issues/920

I thought I have finally resolved this problem, but it seems I have not :( It's great that you have found how can it be reproduced. I will try to fight this.

Btw, the Perfect scrollbar has also some caveats with flexbox, so I'm working on an alternative scroll solution. It should be available in one of the next releases.

nzayatz14 commented 3 years ago

Hey @supersnager, any updates on this?

nzayatz14 commented 3 years ago

Hey @supersnager, I managed to find a work around for this until you get around to your scrolling rewrite.

Pull request can be found here.

Thanks, Nick

supersnager commented 3 years ago

@nzayatz14 I have checked your solution but it unfortunately not works in the simplest test case. What I checked? I applied your fix, opened the Storybook, and set body font-size to 0.875rem like in your repro.

This not works because when the scroll event occurs the <MessageList /> component is not updated, so the code fixed by you is not executing.

But your work led me to some other ideas on how to deal with this issue. IMO the problem still lies in the perfect scrollbar. It can be related to the toInt function which is often used there to get the pixel size from strings. Values passed to this function are from getBoundedClientRect and can be floating numbers, so they are truncated to an integer by parseInt. In some circumstances, this can lead to wrong size calculation.

Please be patient, I need to do more tests on different browsers and devices.