-
Notifications
You must be signed in to change notification settings - Fork 427
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
accomodating content insets and content container padding in visible … #196
Conversation
src/core/RecyclerListView.tsx
Outdated
@@ -107,7 +107,7 @@ export interface RecyclerListViewProps { | |||
|
|||
//For all props that need to be proxied to inner/external scrollview. Put them in an object and they'll be spread | |||
//and passed down. For better typescript support. | |||
scrollViewProps?: object; | |||
scrollViewProps?: ScrollViewProperties; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Web build will fail here
src/core/RecyclerListView.tsx
Outdated
@@ -314,6 +316,26 @@ export default class RecyclerListView extends React.Component<RecyclerListViewPr | |||
// rowRenderer, | |||
// ...props, | |||
// } = this.props; | |||
|
|||
if (!this._derivedContentInsets && this.props.scrollViewProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a utility and move to componentWillReceiveProps.
Render is called multiple times in recyclerview internally.
src/core/RecyclerListView.tsx
Outdated
@@ -40,7 +40,7 @@ import ItemAnimator, { BaseItemAnimator } from "./ItemAnimator"; | |||
import ScrollComponent from "../platform/reactnative/scrollcomponent/ScrollComponent"; | |||
import ViewRenderer from "../platform/reactnative/viewrenderer/ViewRenderer"; | |||
import { DefaultJSItemAnimator as DefaultItemAnimator } from "../platform/reactnative/itemanimators/defaultjsanimator/DefaultJSItemAnimator"; | |||
import { Platform } from "react-native"; | |||
import { Platform, ScrollViewProperties, Insets } from "react-native"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define this type yourself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this type also not available for web?
src/core/RecyclerListView.tsx
Outdated
if (!this._derivedContentInsets && this.props.scrollViewProps) { | ||
this._derivedContentInsets = this.props.scrollViewProps.contentInset ? this.props.scrollViewProps.contentInset : { top: 0, left: 0, right: 0, bottom: 0 }; | ||
|
||
if (this.props.scrollViewProps.contentContainerStyle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contentContainerStyle can also come in props
src/core/RecyclerListView.tsx
Outdated
* Use this prop to provide padding around list content instead of | ||
* using padding/margin in contentContainerStyle prop of scroll view. | ||
*/ | ||
contentInsets?: EdgeInsets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this conflict with existing ScrollView prop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just take paddingTop etc directly which we can override in give contentContainerStyle.
Also will need to be handled in web version
src/core/RecyclerListView.tsx
Outdated
@@ -319,6 +335,14 @@ export default class RecyclerListView extends React.Component<RecyclerListViewPr | |||
ref={(scrollComponent) => this._scrollComponent = scrollComponent as BaseScrollComponent | null} | |||
{...this.props} | |||
{...this.props.scrollViewProps} | |||
{...{ | |||
contentContainerStyle: { | |||
paddingTop: this._contentInsets.top, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All are top
, typo
src/core/RecyclerListView.tsx
Outdated
@@ -319,6 +335,14 @@ export default class RecyclerListView extends React.Component<RecyclerListViewPr | |||
ref={(scrollComponent) => this._scrollComponent = scrollComponent as BaseScrollComponent | null} | |||
{...this.props} | |||
{...this.props.scrollViewProps} | |||
{...{ | |||
contentContainerStyle: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's resolve conflicts if dev already sends this prop
Using existing prop to deal with padding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…row calculations