BugiDev / react-native-calendar-strip

Easy to use and visually stunning calendar component for React Native.
MIT License
936 stars 325 forks source link

add scrollerPaging prop #254

Closed matthewwo closed 3 years ago

matthewwo commented 3 years ago

Addressing #215. scrollerPaging will only work if scrollable is also set to true.

peacechen commented 3 years ago

Thanks @prankymat for adding this feature.

I recommend moving the pagingEnabled ternary logic outside of the JSX for maintainability's sake. While it's convenient to place it inline, it's not as clear what the intent is.

matthewwo commented 3 years ago

thanks @peacechen for the advice! moved the ternary outside of JSX, is this better, or should we get rid of the ternary completely like this?

let scrollViewProps = {
  showsHorizontalScrollIndicator: false,
  contentContainerStyle: {paddingRight: this.state.itemWidth / 2}
}

if (this.props.pagingEnabled) {
  Object.assign(scrollViewProps, {
    decelerationRate: 0,
    snapToInterval: this.state.itemWidth * this.state.numVisibleItems
  })
}
peacechen commented 3 years ago

Thanks @prankymat Your change is good. What do you think about a clearer demarcation between the base scrollViewProps and the paging props like this:

    const pagingProps = this.props.pagingEnabled ? {
        decelerationRate: 0,
        snapToInterval: this.state.itemWidth * this.state.numVisibleItems
    } : {};

    return (
      <View
        style={{ height: this.state.itemHeight, flex: 1 }}
        onLayout={this.onLayout}
      >
        <RecyclerListView
          ref={rlv => this.rlv = rlv}
          layoutProvider={this.state.layoutProvider}
          dataProvider={this.state.dataProvider}
          rowRenderer={this.rowRenderer}
          extendedState={this.props.renderDayParams}
          initialRenderIndex={this.props.initialRenderIndex}
          onVisibleIndicesChanged={this.onVisibleIndicesChanged}
          isHorizontal
          externalScrollView={this.props.externalScrollView}
          scrollViewProps={
                showsHorizontalScrollIndicator: false,
                contentContainerStyle: {paddingRight: this.state.itemWidth / 2},
                ...pagingProps
          }
        />
      </View>

Either way is ok. To me that is clearer at a glance which are the boilerplate scrollView props and which are for paging.

matthewwo commented 3 years ago

totally agree with you, will push a change!

peacechen commented 3 years ago

Thanks @prankymat This will be included in the next release