dingquan / codepath-twitter-app-v2

enhanced version
1 stars 0 forks source link

Please review my app #1

Open dingquan opened 10 years ago

dingquan commented 10 years ago

Twitter app v2 submitted. Please review. The favorite and retweet button are implemented but has bugs. so that feature is half done.

/cc @nesquena @thecodepath

vibhorB commented 10 years ago

Decent work Quan. A few notes after checking out the code:

Here's a detailed Project 4 Feedback Guide here which covers the most common issues with this submitted project. Read through the feedback guide point-by-point to determine how you might be able to improve your submission.

This week (Week 5), we are going to cover the last major piece to the Android puzzle and that is using the hardware and SDK components such as the camera, photo gallery, location, maps, etc. After that, Week 6 and week 7 we will be covering a few important intermediate topics such as more about styling and animation as well as testing.

Following the bootcamp, we are going to have a demo day to celebrate the progress you've all made with our next batch of Android students and multiple companies attending to see the group projects that you all have built. We are going to help however we can over the next few weeks to get the team project apps in shape for that.

dingquan commented 10 years ago

Thanks for the review. A couple of question/comments:

  1. the view pager is not gingerbread compatible? I didn't know that. I thought it is since the package for ViewPager says v4.view.ViewPager
  2. I DID add indeterminate progress bar as I indicated in the completed stories. It just happens that the network loading is super fast so you can barely see it happening. In the walkthrough GIF file, it showed for about less than half a second when I was scrolling.
  3. I DID try to refactor the code to share comment code between the fragments. Both HomeTimelineFragment and MentionsTimelineFragment extends from TweetsListFragment which has empty refreshTimeline() and fetchMoreTimeline() that are overridden in the subclasses. The subclasses only have onCreate, onCreateView and the two overridden methods, everything else is in the base class. Could you please clarify what else can be refactored to be in the base class?
vibhorB commented 10 years ago

Hi Quan

I am really sorry for 1st 2 points you mentioned, just skipped my mind that you added ViewPager and not FragmentTabListener. I checked code again and was able to find progress bar support, I couldnt see it anywhere in GIF. I have corrected the comments, sorry again for that.

For the 3rd point, I saw that you have done a good job creating parent and child classes for fragment. My comment was specifically for refreshTimeline and fetchMoreTimeline. They both essentially do the same thing with different parameters. So I think that part can be improved upon more. But that is just about coding style, I knew you did a good job in creating base class and extracting most of the functionality there.

Thanks Vibhor

dingquan commented 10 years ago

the progress bar can be seen at frame 92 and 93 of the GIF. :) I've taken a couple of snapshots and put them under the root of the repo: twitter_snap1.png and twitter_snap2.png