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

Solution: Compability with react-custom-scrollbars #692

Closed
5angel opened this issue Jun 2, 2017 · 16 comments
Closed

Solution: Compability with react-custom-scrollbars #692

5angel opened this issue Jun 2, 2017 · 16 comments

Comments

@5angel
Copy link
Contributor

5angel commented Jun 2, 2017

After a long discussion in #143 I also took a shot at this problem and arrived to the following solution:

import React, { Component } from 'react';
import { Scrollbars } from 'react-custom-scrollbars';
import { List } from 'react-virtualized';

const SCROLL_DIRECTION_BACKWARD = -1;
const SCROLL_DIRECTION_FORWARD = 1;
const SCROLL_POSITION_CHANGE_REASON_OBSERVED = 'observed';

const listStyle = {
  overflowX: false,
  overflowY: false,
};

export default class SmartList extends Component {
  _getScrollDirection(scrollTop) {
    const {
      scrollTop: oldScrollTop,
      scrollDirectionVertical,
    } = this.List.Grid.state;

    if (scrollTop !== oldScrollTop) {
      return scrollTop > oldScrollTop ?
        SCROLL_DIRECTION_FORWARD :
        SCROLL_DIRECTION_BACKWARD;
    }

    return scrollDirectionVertical;
  }

  handleScroll = ({ target }) => {
    const {
      scrollTop,
      scrollLeft,
    } = target;

    const { Grid: grid } = this.List;

    grid._debounceScrollEnded();

    const scrollDirectionVertical = this._getScrollDirection(scrollTop);
    const totalColumnsWidth = grid._columnSizeAndPositionManager.getTotalSize();
    const totalRowsHeight = grid._rowSizeAndPositionManager.getTotalSize();

    grid.setState({
      isScrolling: true,
      scrollDirectionVertical,
      scrollTop,
      scrollPositionChangeReason: SCROLL_POSITION_CHANGE_REASON_OBSERVED,
    });

    grid._invokeOnScrollMemoizer({ scrollLeft, scrollTop, totalColumnsWidth, totalRowsHeight });
  };

  List = null;

  render() {
    const { width, height } = this.props;

    return (
      <Scrollbars
        autoHide
        style={{ width, height }}
        onScroll={this.handleScroll}
      >
        <List
          {...this.props}
          ref={instance => (this.List = instance)}
          style={listStyle}
        />
      </Scrollbars>
    );
  }
}

You can see that I've pretty much rewritten the _onScroll method in https://github.com/bvaughn/react-virtualized/blob/master/source/Grid/Grid.js#L1110

The simplest way to make this work will be to add a way to bypass the container node check in _onScroll.

@bvaughn what do you think?

@5angel 5angel changed the title Solved: Compability with react-custom-scrollbars Solution: Compability with react-custom-scrollbars Jun 2, 2017
@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

I don't understand this comment on #143:

@bvaughn can you maybe add a second parameter to _onScroll that let you pass event from another container?

_onScroll (and _invokeOnScrollMemoizer) are private- not intended to be called by external code.

I guess Grid could be refactored to add a new handleScroll({ scrollLeft, scrollTop }) method that _onScroll calls internally but could also be called externally. This would avoid the node-check.

@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

If you'd like to submit a PR I'll review it.

@5angel
Copy link
Contributor Author

5angel commented Jun 2, 2017

@bvaughn ah, yes, you're right about that one. brb 👍

@5angel
Copy link
Contributor Author

5angel commented Jun 2, 2017

@bvaughn I'm a bit concerned that test don't pass even without my changes 🤔

@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

Did you yarn install or npm install ?

@5angel
Copy link
Contributor Author

5angel commented Jun 2, 2017

This is a fresh system, so I installed latest node an run npm install. And it failed too, yeah. Should I try yarn?

@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

Use yarn install

@5angel
Copy link
Contributor Author

5angel commented Jun 2, 2017

@bvaughn 👌 Yup, it works now, thanks.

@bvaughn
Copy link
Owner

bvaughn commented Jun 2, 2017

😅 Glad to hear

@bvaughn bvaughn closed this as completed Jun 3, 2017
@alexpyzhianov
Copy link

alexpyzhianov commented Oct 25, 2017

For anyone who struggles with handleScrollEvent and doesn't understand how it is related to the code snippet above, here how it all looks with the new method. @5angel tons of 👍 to you

import React, { Component } from 'react';
import { Scrollbars } from 'react-custom-scrollbars';
import { List } from 'react-virtualized';

const listStyle = {
  overflowX: false,
  overflowY: false,
};

export default class SmartList extends Component {
  handleScroll = ({ target }) => {
    const { scrollTop, scrollLeft } = target;

    const { Grid: grid } = this.List;

    grid.handleScrollEvent({ scrollTop, scrollLeft });
  }

  List = null;

  render() {
    const { width, height } = this.props;

    return (
      <Scrollbars
        autoHide
        style={{ width, height }}
        onScroll={this.handleScroll}
      >
        <List
          {...this.props}
          ref={instance => (this.List = instance)}
          style={listStyle}
        />
      </Scrollbars>
    );
  }
}

@evercool
Copy link

Hi, may I ask for help with solution from @alexpyzhianov? The method handleScrollEvent when fires and updates Grid component causes lags on scrolling. The method handleScrollEvent was bombed by scrolling events so I tried to reduce them by calling it only for scrolls of certain height. It wasnt so laggy, but still each call cause lag on scroll. Next step I tried was to call it in setTimeout to force run it in a new thread (I was desperated) - same result. Any ideas?

@vbrvk
Copy link

vbrvk commented Apr 5, 2018

@evercool have you found solution?

@Justin-ZS
Copy link

Justin-ZS commented May 23, 2018

Maybe another solution, it works for me:

import React from 'react';
import { Scrollbars } from 'react-custom-scrollbars';
import { List, AutoSizer } from 'react-virtualized';

export class DemoList extends React.Component {
  handleScroll = (e) => {
    this.list.Grid._onScroll(e);
  }
  componentDidMount = () => {
    this.list.Grid._scrollingContainer = this.scroll.view;
  }
  render() {
    return (
        <AutoSizer>
          {({ height, width }) => (
            <Scrollbars ref={node => this.scroll = node} onScroll={this.handleScroll} style={{ height, width }}>
              <List
                {...this.props}
                style={{ overflowX: 'visible', overflowY: 'visible' }}
                ref={node => this.list = node}
                height={height}
              />
            </Scrollbars>
          )}
        </AutoSizer>
    );
  }
}

@anstosa
Copy link

anstosa commented Jun 30, 2018

@evercool @borovik96 I'm having a similar problem. Trying to use react-custom-scrollbars instead of ScrollSync to avoid lag problem but the onScroll callback is called for API updates as well so on scroll, the natural event fires, which I use to call scrollTop(y) on the partner Scrollbars instance, but then it triggers onScroll on the other instance and fires it back, etc etc. react-custom-scrollbars does not differentiate API events from user events unfortunately... Any ideas?

@serle
Copy link

serle commented Aug 6, 2018

Can this technique be used with the table and would it let the headers be fixed and only scroll the data?

goldylucks added a commit to zenprotocol/zenwallet that referenced this issue Jan 24, 2019
- npm: added `react-virtualized`
- npm: added `highlight.js`

at the moment I disabled highlighting the logs since it breaks the UI. The horizontal scrollbar isn't styled. This can be probably be fixed by following [this discussion](bvaughn/react-virtualized#692)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants