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

accomodating content insets and content container padding in visible … #196

Merged
merged 7 commits into from
Jun 19, 2018

Conversation

rajatgupta26
Copy link
Collaborator

…row calculations

@@ -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;
Copy link
Collaborator

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

@@ -314,6 +316,26 @@ export default class RecyclerListView extends React.Component<RecyclerListViewPr
// rowRenderer,
// ...props,
// } = this.props;

if (!this._derivedContentInsets && this.props.scrollViewProps) {
Copy link
Collaborator

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.

@@ -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";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define this type yourself

Copy link
Collaborator Author

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?

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) {
Copy link
Collaborator

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

* Use this prop to provide padding around list content instead of
* using padding/margin in contentContainerStyle prop of scroll view.
*/
contentInsets?: EdgeInsets;
Copy link
Collaborator

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?

Copy link
Collaborator

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

@@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All are top, typo

@@ -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: {
Copy link
Collaborator

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

Copy link
Collaborator

@naqvitalha naqvitalha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@naqvitalha naqvitalha merged commit d3f0472 into master Jun 19, 2018
@naqvitalha naqvitalha deleted the insetsFix branch June 19, 2018 09:42
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

Successfully merging this pull request may close these issues.

2 participants