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.

Sign up for GitHub

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

Jump to bottom

Pull down to refresh implemented #132

Merged
merged 32 commits into from Jul 31, 2017
Merged

Pull down to refresh implemented #132

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
f455e30
Merge pull request #1 from gitpoint/master
SammyIsra Jul 17, 2017
9e091a8
Added react-native-cli to avoid onboarding issues in the future
SammyIsra Jul 20, 2017
f52e2f6
Pull down to refresh implemented in profile page
SammyIsra Jul 20, 2017
90c8d76
Pull down to refresh implemented in Repository page. Closes #103.
SammyIsra Jul 20, 2017
8e285b4
Merge branch 'master' into master
SammyIsra Jul 20, 2017
5eb2b51
Fixed some merge issues
SammyIsra Jul 20, 2017
870ea3c
Fixing some more merging issues
SammyIsra Jul 20, 2017
9a86228
Forgot to add myself as a contributor.
SammyIsra Jul 20, 2017
08548e5
Fetching user data and fetching repository data have been moved into …
SammyIsra Jul 21, 2017
e02fe90
Merge branch 'master' into master
SammyIsra Jul 22, 2017
f308a7b
Fixed some merge issues.
SammyIsra Jul 22, 2017
36d8cf7
Renamed the function that fetches the information to be a bit more de…
SammyIsra Jul 22, 2017
5d7736b
Merge branch 'master' into master
SammyIsra Jul 22, 2017
088dc12
Again, fixing my own merge issues. Doing it here with code highlight …
SammyIsra Jul 23, 2017
9d97a3a
Merge branch 'master' into master
SammyIsra Jul 24, 2017
7c11120
Fixed some merge issues, _again_
SammyIsra Jul 24, 2017
f8506e7
Added pull to refresh to the non auth profile screen. Other minor imp…
SammyIsra Jul 24, 2017
438a87f
Fixing indentation inside of `<ScrollView />`
SammyIsra Jul 24, 2017
f7da7a5
Removing extra newline
SammyIsra Jul 24, 2017
899d028
Adding a newline between our components and vendor components.
SammyIsra Jul 24, 2017
a17a572
Added myself as a contributor. Code and Bug Reports
SammyIsra Jul 24, 2017
88eee32
Valid NPM versions need to include three parts (x.y.z)
SammyIsra Jul 27, 2017
15d685b
Removed the double ScrollView. Parallax can now take in refreshContro…
SammyIsra Jul 27, 2017
ec7c9ef
Pull down to refresh now in Issue/PR page (comment list)
SammyIsra Jul 27, 2017
f484cd8
Merge branch 'master' into master
SammyIsra Jul 27, 2017
442690c
Fixed some merging issues and yarn lockfile
SammyIsra Jul 27, 2017
203cc2b
Organizations are now refreshable
SammyIsra Jul 27, 2017
f6ed3ba
Better behavior on the AuthUser pull to refresh
SammyIsra Jul 27, 2017
02c5e67
Even better experience when refreshing AuthProfile. Now we have a fla…
SammyIsra Jul 27, 2017
3e907f9
Refreshing on Issues (specifically) works again
SammyIsra Jul 28, 2017
5d639c1
Merge branch 'master' into master
SammyIsra Jul 28, 2017
66ad3e3
Used '!!' instead of '|| false' as per suggestion
SammyIsra Jul 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Expand Up @@ -63,6 +63,7 @@
"babel-eslint": "^7.1.1",
"babel-jest": "18.0.0",
"babel-plugin-module-resolver": "^2.7.1",
"babel-plugin-transform-inline-environment-variables": "^6.8.0",
"babel-preset-flow": "^6.23.0",
"babel-preset-react-native": "1.9.1",
"eslint": "^3.19.0",
Expand All @@ -79,8 +80,8 @@
"lint-staged": "^3.2.6",
"pre-commit": "^1.2.2",
"prettier": "^1.3.1",
"react-test-renderer": "16.0.0-alpha.6",
"babel-plugin-transform-inline-environment-variables": "^6.8.0"
"react-native-cli": "^2.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in here? Maybe I just missed it, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

My machine was having problems with running yarn commands that called on react-native. The problem was with npm itself, and a workaround listed was to include react-native-cli as a dev dependency.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me then. =]

"react-test-renderer": "16.0.0-alpha.6"
},
"jest": {
"preset": "react-native"
Expand Down
163 changes: 89 additions & 74 deletions src/auth/screens/auth-profile.screen.js
View file
Open in desktop
@@ -1,6 +1,6 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
import { StyleSheet } from 'react-native';
import { StyleSheet, ScrollView, RefreshControl } from 'react-native';
import { ListItem } from 'react-native-elements';
import Communications from 'react-native-communications';

Copy link
Member

Choose a reason for hiding this comment

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

Keep this newline for consistency. We're separating external modules from internal components.

Expand Down Expand Up @@ -60,90 +60,105 @@ class AuthProfile extends Component {
return url.substr(0, prefix.length) === prefix ? url : `http://${url}`;
};

refreshProfile = () => {
this.props.getUser();
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract this to a separate method (for example: getUserDetails) and then call getUserDetails in componentDidMount as well as in the ScrollView refreshControl prop?

this.props.getOrgs();
};

render() {
const { user, orgs, isPendingUser, isPendingOrgs, navigation } = this.props;
const loading = isPendingUser || isPendingOrgs;

return (
<ViewContainer>
{loading && <LoadingContainer animating={loading} center />}

{!loading &&
<ParallaxScroll
renderContent={() =>
<UserProfile
type="user"
initialUser={user}
user={user}
navigation={navigation}
/>}
stickyTitle={user.login}
>
{user.bio &&
user.bio !== '' &&
<SectionList title="BIO">
<ScrollView
refreshControl={
<RefreshControl
refreshing={loading}
onRefresh={this.refreshProfile}
/>
}
>
{loading && <LoadingContainer animating={loading} center />}

{!loading &&
<ParallaxScroll
renderContent={() =>
<UserProfile
type="user"
initialUser={user}
user={user}
navigation={navigation}
/>}
stickyTitle={user.login}
>
{user.bio &&
Copy link
Member

Choose a reason for hiding this comment

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

We can use lodash.get to perform this kind of tests.
It should be something like const userHasBio = get(user, ['bio'], '').length; so you can omit the subsequent checks

user.bio !== '' &&
<SectionList title="BIO">
<ListItem
subtitle={user.bio}
subtitleStyle={styles.listSubTitle}
hideChevron
/>
</SectionList>}

<SectionList
title="EMAIL"
noItems={!user.email || user.email === ''}
noItemsMessage={'No email associated with account'}
>
<ListItem
subtitle={user.bio}
title="Email"
titleStyle={styles.listTitle}
leftIcon={{
name: 'mail',
color: colors.grey,
type: 'octicon',
}}
subtitle={user.email}
subtitleStyle={styles.listSubTitle}
hideChevron
onPress={() =>
Communications.email([user.email], null, null, 'Hi!', '')}
underlayColor={colors.greyLight}
/>
</SectionList>}
</SectionList>
Copy link
Member

Choose a reason for hiding this comment

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

This email section can be removed as it's included in EntityInfo 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah a bit of a mistake when merging... Last commit should have removed it I think, a second swing at merging your master into my branch


<SectionList
title="EMAIL"
noItems={!user.email || user.email === ''}
noItemsMessage={'No email associated with account'}
>
<ListItem
title="Email"
titleStyle={styles.listTitle}
leftIcon={{
name: 'mail',
color: colors.grey,
type: 'octicon',
}}
subtitle={user.email}
subtitleStyle={styles.listSubTitle}
onPress={() =>
Communications.email([user.email], null, null, 'Hi!', '')}
underlayColor={colors.greyLight}
/>
</SectionList>

<SectionList
title="WEBSITE"
noItems={!user.blog || user.blog === ''}
noItemsMessage={'No website associated with account'}
>
<ListItem
title="Website"
titleStyle={styles.listTitle}
leftIcon={{
name: 'link',
color: colors.grey,
type: 'octicon',
}}
subtitle={user.blog}
subtitleStyle={styles.listSubTitle}
onPress={() => Communications.web(this.getUserBlog(user.blog))}
underlayColor={colors.greyLight}
/>
</SectionList>

<SectionList
title="ORGANIZATIONS"
noItems={orgs.length === 0}
noItemsMessage={'No organizations'}
>
{orgs.map(item =>
<UserListItem
key={item.id}
user={item}
navigation={navigation}
<SectionList
title="WEBSITE"
noItems={!user.blog || user.blog === ''}
noItemsMessage={'No website associated with account'}
>
<ListItem
title="Website"
titleStyle={styles.listTitle}
leftIcon={{
name: 'link',
color: colors.grey,
type: 'octicon',
}}
subtitle={user.blog}
subtitleStyle={styles.listSubTitle}
onPress={() =>
Communications.web(this.getUserBlog(user.blog))}
underlayColor={colors.greyLight}
/>
)}
</SectionList>
</ParallaxScroll>}
</SectionList>

<SectionList
title="ORGANIZATIONS"
noItems={orgs.length === 0}
Copy link
Member

Choose a reason for hiding this comment

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

!orgs.length is better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it be more readable? In the end, the result would be the same but in my opinion, .length=== 0 is more readable than !.length

noItemsMessage={'No organizations'}
>
{orgs.map(item =>
<UserListItem
key={item.id}
user={item}
navigation={navigation}
/>
)}
</SectionList>
</ParallaxScroll>}
</ScrollView>
</ViewContainer>
);
}
Copy link
Member

Choose a reason for hiding this comment

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

This has got quite a few conflicts and is using a pretty old version of the file. For example, most of these SectionList's can be replaced with the <EntityInfo /> component.

Expand Down