-
Notifications
You must be signed in to change notification settings - Fork 1
Settings following #24
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
base: master
Are you sure you want to change the base?
Conversation
Wondering: keep all actions at the App.js level or move to their containers? Getting awfully close to considering something redux-y... I really like https://github.com/jxnblk/refunk maybe a nice slimmed down alternative. Probably nothing to worry about now though. |
Actually, this is still clean af. Let's keep it for a bit. Might get a little ridiculous using that many render props... but we're like... almost done with v1. v2 might need more organization for following. |
There are a LOT of improvements here and functionality BUT.... It looks like while you're refactoring the posts query stuff directly in master you ended up breaking the followers code that was in this repo. We probably don't want to merge this before you push some fixes to master about that code and then we remerge. |
NOT DIRECTLY IN MASTER |
}; | ||
} | ||
|
||
async componentDidMount() { | ||
try { | ||
const archive = await new global.DatArchive(urlEnv()); | ||
const archiveInfo = await archive.getInfo(); | ||
const allPosts = await this.getAllPosts(archive); | ||
const results = await this.refreshPosts(archive); |
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.
I think there's a faulty merge going on here, starting with this line. We want the getAllPosts stuff
const userData = await this.getUserInfo(archive); | ||
|
||
this.setState({ | ||
correctBrowser: true, | ||
posts: allPosts, |
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.
We need this
* returns all of their posts. | ||
* It does not setState. | ||
*/ | ||
getAllPosts = async archive => { |
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.
refreshPosts is now getAllPosts
* Exact same as the above function, but | ||
* also sets state. | ||
*/ | ||
getAllPostsSaved = async archive => { |
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.
This is a new function. Similar to the old refreshPosts, but it also does setState
@@ -94,9 +94,7 @@ class App extends Component { | |||
return JSON.parse(postResponse); | |||
}); | |||
const results = await Promise.all(promises); | |||
this.setState({ |
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.
Need this
this.state.postDisplay === 'theirs' | ||
? this.state.theirPosts | ||
: this.state.posts | ||
)} | ||
isOwner={this.state.isOwner} | ||
getPosts={this.getAllPostsSaved} |
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.
One of those new functions referenced here
@@ -0,0 +1,8 @@ | |||
const profileContents = userData => `{ |
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.
This could be refactored to something like fileContents.js
const profileContents = userData =>
JSON.stringify({
avatar: userData.avatar,
bio: userData.bio,
name: userData.name,
follows: userData.follows
});
export default profileContents;
This reverts commit 4191638.
i dont know whats going on in this branch anymore, but p sure its stuxnet |
let's create another branch instead of merging this |
turning this into a PR so we can comment on it