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

Made the about us page dynamic. Loads details from a json file. #53

Merged
merged 2 commits into from
Sep 30, 2017
Merged

Made the about us page dynamic. Loads details from a json file. #53

merged 2 commits into from
Sep 30, 2017

Conversation

Kartikdot
Copy link
Contributor

@Kartikdot Kartikdot commented Aug 29, 2017 •
edited

Team page; static data
Fixes #47
Please review the changes. Gradle version has not been updated.

Swati4star
Swati4star reviewed Sep 3, 2017
View reviewed changes
app/src/main/java/nsit/app/com/nsitapp/AboutUs2.java Outdated
@@ -0,0 +1,98 @@
package nsit.app.com.nsitapp;
Copy link
Member

Choose a reason for hiding this comment

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

Why have you changed the file name to AboutUs2? Let it be AboutUs only.

Swati4star
Swati4star reviewed Sep 3, 2017
View reviewed changes
app/src/main/java/nsit/app/com/nsitapp/AboutUs2.java Outdated
aboutUsListView.setAdapter(mAdapter);
aboutUsListView.addFooterView(footerView);
TextView rep = (TextView) rootView.findViewById(R.id.GoToRepo);
TextView con = (TextView) rootView.findViewById(R.id.cont);
Copy link
Member

Choose a reason for hiding this comment

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

Make the variable name more informative. rep & con doesn't actually tell, what these are for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These variable names were picked up from the previous code. I'll change them.

Swati4star
Swati4star reviewed Sep 3, 2017
View reviewed changes
app/src/main/java/nsit/app/com/nsitapp/MainActivity.java
@@ -61,6 +61,7 @@ protected void onCreate(Bundle savedInstanceState) {
requestWindowFeature(Window.FEATURE_INDETERMINATE_PROGRESS);
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
appContext=getApplicationContext();
Copy link
Member

Choose a reason for hiding this comment

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

What is the need of this? I don't think you are using it anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being used to get the json file using the getResources() method in class utility.

Swati4star
Swati4star reviewed Sep 3, 2017
View reviewed changes
app/src/main/java/nsit/app/com/nsitapp/aboutUs_loader.java Outdated
* Created by Kartik Kwatra on 27-08-2017.
*/

public class aboutUs_loader extends AsyncTaskLoader<List<AboutUsMember>> {
Copy link
Member

Choose a reason for hiding this comment

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

Class names should be like this..
"AboutUsLoader"

Swati4star
Swati4star reviewed Sep 3, 2017
View reviewed changes
app/src/main/res/layout/aboutus_activity.xml Outdated
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

aboutUs is a fragment. Isn't it?

@Swati4star
Copy link
Member

The functionality is complete overall. But there are a few changes, that needs to be taken care of, before merging. Also, Can you explain the reason for adding so many Java files? BAsically, tell the functionality of each one.

@Kartikdot
Copy link
Contributor Author

Thank you for the review. I have noted the changes to be made.
On it! I'll also explain the functionality of the java files after making the required changes.

@Kartikdot
1. Renamed class files
28d32ee 
2. Renamed the rep and con variables to goToRepo and contribute respectively
3. Renamed the layout files since they are fragments
@Kartikdot
Copy link
Contributor Author

Made the changes as per the review.

4 new Java files have been added and 1 modified. Here i list the functionality of each.

  1. AboutUs - This is the main fragment. It sets the layout, initializes the adapter for the listview. It also implements Loader callbacks and calls the initLoader() method.

  2. AboutUsMember - This class defines the properties of a team member which will be displayed on the about us page. Objects of this class are used to populate the list view.

  3. AboutUsLoader - This is a custom loader. It extends the AsyncTaskLoader. I made a custom loader instead of directly using AsyncTaskLoader since in case the user rotates the phone, a new instance of AsyncTask would be created and a new thread would be started. This would result in wastage of memory. Also, repeated changes in orientation might start too many threads resulting the app to crash.

  4. AboutUsUtility - This class fetches the list of team members from the json file and generates an ArrayList of the type AboutUsMember. Functions of this class are called on a separate thread using the loader.

  5. MemberAdapter - This class populates the list view using the ArrayList of AboutUsMember.

@Kartikdot
Copy link
Contributor Author

Fixes #47

@Kartikdot
Copy link
Contributor Author

Please merge #53

@Swati4star Swati4star merged commit 28d32ee into NSITonline:master Sep 30, 2017
@Swati4star Swati4star mentioned this pull request Sep 30, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Swati4star Swati4star Swati4star left review comments

Assignees
No one assigned
Labels
under-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Kartikdot @Swati4star