sakhnyuk / rc-scrollbars

React scrollbars component
https://rc-scrollbars.vercel.app/
MIT License
145 stars 14 forks source link

className support #8

Closed martpie closed 3 years ago

martpie commented 3 years ago

Thank you for taking over the project 👏

I tried to move my project to rc-scrollbars, but it seems I cannot set className to <Scrollbars />, would it be possible to add it?

sakhnyuk commented 3 years ago

Hi, @martpie

I think you can try to add your components with classNames by special props. But don't forget to forward the props object from the handler to your component. So example below:


class CustomScrollbars extends Component {
  render() {
    return (
      <Scrollbars
        renderTrackHorizontal={props => <div {...props} className="track-horizontal"/>}
        renderTrackVertical={props => <div {...props} className="track-vertical"/>}
        renderThumbHorizontal={props => <div {...props} className="thumb-horizontal"/>}
        renderThumbVertical={props => <div {...props} className="thumb-vertical"/>}
        renderView={props => <div {...props} className="view"/>}>
        {this.props.children}
      </Scrollbars>
    );
  }
}
martpie commented 3 years ago

I think there is an issue here:

https://github.com/sakhnyuk/rc-scrollbars/blob/19fe0faf970e2cf30982febf0c7614451ae23859/src/Scrollbars/index.tsx#L692

shouldn't it forwards props's className as well?

martpie commented 3 years ago

Actually, I wonder if it should not forward everything ({...rest}?) from <Scrollbars>, attaching an onScroll on <Scrollbar> does not work either

Tomassito commented 3 years ago

@martpie do you want to apply the className to the Container (outer) or View (inner) element? Because in the initial question you mention outer and now you refer to the inner...

sakhnyuk commented 3 years ago

I think there is an issue here:

https://github.com/sakhnyuk/rc-scrollbars/blob/19fe0faf970e2cf30982febf0c7614451ae23859/src/Scrollbars/index.tsx#L692

shouldn't it forwards props's className as well?

That's right. I will check how props passing from renderView. ClassNames should be merged I think

martpie commented 3 years ago

Maybe I should provide more context 😄

So here's my component:

import React, { useCallback } from 'react';
import Scrollbars from 'react-custom-scrollbars';

import styles from './CustomScrollbar.module.css';

interface Props {
  className: string;
  onScroll: React.UIEventHandler<any>;
}

const CustomScrollbar: React.FC<Props> = (props) => {
  const getRenderView = useCallback((props: any) => {
    console.log(props);
    return <div {...props} className={styles.renderView} />;
  }, []);

  const getTrackVertical = useCallback((props: any) => {
    return <div {...props} className={styles.verticalTrack} />;
  }, []);

  const getThumbVertical = useCallback((props: any) => {
    return <div {...props} className={styles.verticalThumb} />;
  }, []);

  return (
    <Scrollbars
      className={props.className}
      onScroll={props.onScroll}
      renderView={getRenderView}
      renderTrackVertical={getTrackVertical}
      renderThumbVertical={getThumbVertical}
      autoHide
      autoHideTimeout={1000}
    >
      {props.children}
    </Scrollbars>
  );
};

export default CustomScrollbar;

Nothing to fancy there, this was using react-custom-scrollbars. This is what the DOM looks like:

Screen Shot 2020-12-28 at 12 44 06

tracks-list-body__xxx is my component props.className.

Now after switcing to rc_scrollbars, the DOM looks like the following:

Screen Shot 2020-12-28 at 12 46 17

So indeed, the outer wrapper looks good, and the inner seems to not accept className.

Thank you for taking the time for looking into it, I really appreciate 🙌

sakhnyuk commented 3 years ago

Actually, I wonder if it should not forward everything ({...rest}?) from <Scrollbars>, attaching an onScroll on <Scrollbar> does not work either

ScrollBars has its own list of props. It is using right from component and you can check that. Events tracking by native API (addEventListener and removeEvent...) and onScroll prop calling there.

Custom view and tracks are only about style.

sakhnyuk commented 3 years ago

@martpie alright! Thank you for your example. 👍 I can fix that today.

Tomassito commented 3 years ago

@martpie Thanks for the example (they always work best :) ) . Speaking of className - do you not get a TS error when you provide this prop to the Scrollbars component??

martpie commented 3 years ago

@Tomassito actually yes, I think I used a @ts-ignore here and it's still working. (It was valid in react-custom-scrollbars at least)

edit: yes I can confirm: TS does not accept it, but it works (for the outer wrapper)

sakhnyuk commented 3 years ago

@martpie I deployed v1.0.3 with deleted className prop.

martpie commented 3 years ago

I can confirm it works fine, thanks! 🙌