-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Refactoring People Management UI to use Fragments #3970
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
Merged
hypest
merged 45 commits into
feature/people-management-sync
from
feature/people-management-ui-layer
Apr 18, 2016
Merged
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
f6a4c01
PeopleListFragment introduced
oguzkocer 2d4ab80
Fixes the people list fragment and people management activity layouts
oguzkocer 598c351
PeopleListFragment is now reusable by having a setPeopleList method.
oguzkocer c420774
PeopleManagementActivity now sets & updates PeopleListFragment
oguzkocer dcef47c
PersonDetailFragment introduced
oguzkocer 73dbe5f
PersonActivity is removed as it's no longer used
oguzkocer 3ebabfb
Fixes usage of this instead of getActivity() in PersonDetailFragment
oguzkocer b5dce00
Merge branch 'feature/people-management-sync' into feature/people-man…
oguzkocer a539f3f
Open PersonDetailFragment when a person is selected
oguzkocer 4557dbe
Override onBackPressed so the previous fragment is shown for People M…
oguzkocer 4f94b87
Check isAdded() in setPeopleList for PeopleListFragment
oguzkocer 43bb643
OnPersonSelectedListener introduced to communicate with the activity
oguzkocer f913f58
Instead of setPerson, use newInstance method to pass person details
oguzkocer 9570324
Refresh person detail fragment when the people list is updated
oguzkocer ba5f330
When person details change update the UI
oguzkocer 345e935
Instead of using setPeopleList method, pass the blog id to refresh pe…
oguzkocer 77dd11c
Nullify listener onDetach for PeopleListFragment
oguzkocer a173b85
Instead of keeping a reference to fragments use tags
oguzkocer 4bd521f
Simplify refreshPersonDetails by returning immediately if fragment is…
oguzkocer 45ceb24
Only create people list fragment for the first time (fixes rotation i…
oguzkocer f97b3ec
Fix: Add the fragment tags during transactions
oguzkocer 3c88fa3
First stab at adding slide in-out animation to People Management frag…
oguzkocer d5b29ed
Removing android:interpolator fixes the weirdness in the animation
oguzkocer 05db078
Use the correct container for the fragments for PeopleManagementActivity
oguzkocer efac6dc
Add missing person detail fragment tag
oguzkocer dbcea61
Use XFraction instead of X for animation fragments
oguzkocer 6cd57f4
Implement onAttach(Activity activity) for older devices
oguzkocer 09aee84
Convert PeopleListFragment to ListFragment from a regular Fragment
oguzkocer d6ddf93
Merge branch 'feature/people-management-sync' into feature/people-man…
oguzkocer 16cde86
Use custom sliding layout for fragments instead of activity
oguzkocer 3468e2a
Adds slide_in_from_left & slide_out_to_right animations for fragments
oguzkocer 33eac9b
Adds interpolator to fragment animations to match the activity counte…
oguzkocer 964b907
Refactor SlidingRelativeLayout so the first animation frame is drawn …
oguzkocer 8d30df1
Fix AndroidManifest entry for PeopleManagementActivity
oguzkocer 8ab36c9
Follow the naming conventions for properties in SlidingRelativeLayout
oguzkocer ae4c73d
Convert constant strings to all lowercase to be consistent
oguzkocer 663fcd2
notifyDataSetChanged moved to setPeopleList since it makes more sense…
oguzkocer d2e1afe
onCreate method should be public in PeopleManagementActivity
oguzkocer 9a40740
PeopleManagementActivity small styling & naming issues fixed
oguzkocer ded9310
Adds log for null-check fail in PersonDetailFragment
oguzkocer fa7b198
Adds @SuppressWarnings(UnusedDeclaration) and comment expaning the us…
oguzkocer 2eac0f9
Fixes the fragment animation issue and makes it more like activity ve…
oguzkocer 9298551
Use onCreateAnimator instead of FragmentTransaction as config changes…
oguzkocer 12be72a
Adds a comment explaining the usage of SlidingRelativeLayout and back…
oguzkocer b784058
No need to use SlidingRelativeLayout & background color in people lis…
oguzkocer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
51 changes: 51 additions & 0 deletions
51
WordPress/src/main/java/org/wordpress/android/ui/SlidingRelativeLayout.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package org.wordpress.android.ui; | ||
|
|
||
| import android.content.Context; | ||
| import android.util.AttributeSet; | ||
| import android.view.ViewTreeObserver; | ||
| import android.widget.RelativeLayout; | ||
|
|
||
| public class SlidingRelativeLayout extends RelativeLayout { | ||
|
|
||
| private float mXFraction = 0; | ||
|
|
||
| public SlidingRelativeLayout(Context context) { | ||
| super(context); | ||
| } | ||
|
|
||
| public SlidingRelativeLayout(Context context, AttributeSet attrs) { | ||
| super(context, attrs); | ||
| } | ||
|
|
||
| private ViewTreeObserver.OnPreDrawListener mPreDrawListener = null; | ||
|
|
||
| // This property is used by the objectAnimator for Fragment slide animations. ex: `fragment_slide_in_from_right.xml` | ||
| @SuppressWarnings("UnusedDeclaration") | ||
| public float getXFraction() { | ||
| return mXFraction; | ||
| } | ||
|
|
||
| // This implementation fixes the first frame not being translated: http://trickyandroid.com/fragments-translate-animation/ | ||
| @SuppressWarnings("UnusedDeclaration") | ||
| public void setXFraction(float fraction) { | ||
| mXFraction = fraction; | ||
|
|
||
| if (getWidth() == 0) { | ||
| if (mPreDrawListener == null) { | ||
| mPreDrawListener = new ViewTreeObserver.OnPreDrawListener() { | ||
| @Override | ||
| public boolean onPreDraw() { | ||
| getViewTreeObserver().removeOnPreDrawListener(mPreDrawListener); | ||
| setXFraction(mXFraction); | ||
| return true; | ||
| } | ||
| }; | ||
| getViewTreeObserver().addOnPreDrawListener(mPreDrawListener); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| float translationX = getWidth() * fraction; | ||
| setTranslationX(translationX); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
107 changes: 107 additions & 0 deletions
107
WordPress/src/main/java/org/wordpress/android/ui/people/PeopleListFragment.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| package org.wordpress.android.ui.people; | ||
|
|
||
| import android.app.Activity; | ||
| import android.app.ListFragment; | ||
| import android.content.Context; | ||
| import android.os.Bundle; | ||
| import android.view.LayoutInflater; | ||
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
| import android.widget.AdapterView; | ||
| import android.widget.AdapterView.OnItemClickListener; | ||
|
|
||
| import org.wordpress.android.R; | ||
| import org.wordpress.android.datasets.PeopleTable; | ||
| import org.wordpress.android.models.Person; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class PeopleListFragment extends ListFragment implements OnItemClickListener { | ||
| private static String ARG_LOCAL_TABLE_BLOG_ID = "local_table_blog_id"; | ||
|
|
||
| private int mLocalTableBlogID; | ||
| private OnPersonSelectedListener mListener; | ||
|
|
||
| public static PeopleListFragment newInstance(int localTableBlogID) { | ||
| PeopleListFragment peopleListFragment = new PeopleListFragment(); | ||
| Bundle bundle = new Bundle(); | ||
| bundle.putInt(ARG_LOCAL_TABLE_BLOG_ID, localTableBlogID); | ||
| peopleListFragment.setArguments(bundle); | ||
| return peopleListFragment; | ||
| } | ||
|
|
||
| @Override | ||
| public void onAttach(Context context) { | ||
| super.onAttach(context); | ||
| try { | ||
| mListener = (OnPersonSelectedListener) context; | ||
| } catch (ClassCastException e) { | ||
| throw new ClassCastException(context.toString() + " must implement OnPersonSelectedListener"); | ||
| } | ||
| } | ||
|
|
||
| // We need to override this for devices pre API 23 | ||
| @SuppressWarnings("deprecation") | ||
| @Override | ||
| public void onAttach(Activity activity) { | ||
| super.onAttach(activity); | ||
| try { | ||
| mListener = (OnPersonSelectedListener) activity; | ||
| } catch (ClassCastException e) { | ||
| throw new ClassCastException(activity.toString() + " must implement OnPersonSelectedListener"); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onDetach() { | ||
| super.onDetach(); | ||
| mListener = null; | ||
| } | ||
|
|
||
| @Override | ||
| public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { | ||
| return inflater.inflate(R.layout.people_list_fragment, container, false); | ||
| } | ||
|
|
||
| @Override | ||
| public void onActivityCreated(Bundle savedInstanceState) { | ||
| super.onActivityCreated(savedInstanceState); | ||
|
|
||
| mLocalTableBlogID = getArguments().getInt(ARG_LOCAL_TABLE_BLOG_ID); | ||
| getListView().setOnItemClickListener(this); | ||
| } | ||
|
|
||
| @Override | ||
| public void onResume() { | ||
| super.onResume(); | ||
|
|
||
| refreshPeopleList(); | ||
| } | ||
|
|
||
| public void refreshPeopleList() { | ||
| if (!isAdded()) return; | ||
|
|
||
| List<Person> peopleList = PeopleTable.getPeople(mLocalTableBlogID); | ||
|
|
||
| PeopleAdapter peopleAdapter = (PeopleAdapter) getListAdapter(); | ||
| if (peopleAdapter == null) { | ||
| peopleAdapter = new PeopleAdapter(getActivity(), peopleList); | ||
| setListAdapter(peopleAdapter); | ||
| } else { | ||
| peopleAdapter.setPeopleList(peopleList); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onItemClick(AdapterView<?> parent, View view, int position, long id) { | ||
| if (mListener != null) { | ||
| Person person = (Person) parent.getItemAtPosition(position); | ||
| mListener.onPersonSelected(person); | ||
| } | ||
| } | ||
|
|
||
| // Container Activity must implement this interface | ||
| public interface OnPersonSelectedListener { | ||
| void onPersonSelected(Person person); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,11 @@ | ||
| package org.wordpress.android.ui.people; | ||
|
|
||
| import android.app.Activity; | ||
| import android.app.FragmentManager; | ||
| import android.app.FragmentTransaction; | ||
| import android.os.Bundle; | ||
| import android.support.v7.app.ActionBar; | ||
| import android.support.v7.app.AppCompatActivity; | ||
| import android.view.MenuItem; | ||
| import android.view.View; | ||
| import android.widget.AdapterView; | ||
| import android.widget.ListView; | ||
|
|
||
| import com.android.volley.VolleyError; | ||
|
|
||
|
|
@@ -23,41 +21,33 @@ | |
|
|
||
| import java.util.List; | ||
|
|
||
| public class PeopleManagementActivity extends AppCompatActivity { | ||
|
|
||
| private PeopleAdapter mPeopleAdapter; | ||
| public class PeopleManagementActivity extends AppCompatActivity implements PeopleListFragment.OnPersonSelectedListener { | ||
| private static final String KEY_PEOPLE_LIST_FRAGMENT = "people-list-fragment"; | ||
| private static final String KEY_PERSON_DETAIL_FRAGMENT = "person-detail-fragment"; | ||
|
|
||
| @Override | ||
| public void onCreate(Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
|
|
||
| int localBlogId = BlogUtils.getBlogLocalId(WordPress.getCurrentBlog()); | ||
| Blog blog = WordPress.getBlog(localBlogId); | ||
|
|
||
| ActionBar actionBar = getSupportActionBar(); | ||
| if (actionBar != null) { | ||
| actionBar.setHomeButtonEnabled(true); | ||
| actionBar.setDisplayHomeAsUpEnabled(true); | ||
| } | ||
| setContentView(R.layout.people_management_activity); | ||
|
|
||
| setTitle(R.string.people); | ||
| int localBlogId = BlogUtils.getBlogLocalId(WordPress.getCurrentBlog()); | ||
| Blog blog = WordPress.getBlog(localBlogId); | ||
|
|
||
| if (savedInstanceState == null) { | ||
| PeopleListFragment peopleListFragment = PeopleListFragment.newInstance(localBlogId); | ||
|
|
||
| if (blog != null) { | ||
| ListView listView = (ListView)findViewById(android.R.id.list); | ||
| List<Person> peopleList = PeopleTable.getPeople(localBlogId); | ||
| mPeopleAdapter = new PeopleAdapter(this, peopleList); | ||
| listView.setAdapter(mPeopleAdapter); | ||
|
|
||
| final Activity context = this; | ||
| listView.setOnItemClickListener(new AdapterView.OnItemClickListener() { | ||
| @Override | ||
| public void onItemClick(AdapterView<?> parent, View view, int position, long id) { | ||
| Person person = (Person) parent.getItemAtPosition(position); | ||
| ActivityLauncher.viewPersonDetails(context, person); | ||
| } | ||
| }); | ||
| getFragmentManager().beginTransaction() | ||
| .add(R.id.fragment_container, peopleListFragment, KEY_PEOPLE_LIST_FRAGMENT) | ||
| .commit(); | ||
| } | ||
|
|
||
| if (blog != null) { | ||
| refreshUsersList(blog.getDotComBlogId(), localBlogId); | ||
| } | ||
| } | ||
|
|
@@ -68,6 +58,15 @@ public void finish() { | |
| ActivityLauncher.slideOutToRight(this); | ||
| } | ||
|
|
||
| @Override | ||
| public void onBackPressed() { | ||
| if (getFragmentManager().getBackStackEntryCount() > 0 ){ | ||
| getFragmentManager().popBackStack(); | ||
| } else { | ||
| super.onBackPressed(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean onOptionsItemSelected(final MenuItem item) { | ||
| if (item.getItemId() == android.R.id.home) { | ||
|
|
@@ -77,13 +76,24 @@ public boolean onOptionsItemSelected(final MenuItem item) { | |
| return super.onOptionsItemSelected(item); | ||
| } | ||
|
|
||
| private void refreshUsersList(String dotComBlogId, final int localBlogId) { | ||
| PeopleUtils.fetchUsers(dotComBlogId, localBlogId, new PeopleUtils.Callback() { | ||
| private void refreshUsersList(String dotComBlogId, final int localTableBlogId) { | ||
| PeopleUtils.fetchUsers(dotComBlogId, localTableBlogId, new PeopleUtils.Callback() { | ||
| @Override | ||
| public void onSuccess(List<Person> peopleList) { | ||
| PeopleTable.savePeople(peopleList); | ||
| mPeopleAdapter.setPeopleList(peopleList); | ||
| mPeopleAdapter.notifyDataSetChanged(); | ||
|
|
||
| FragmentManager fragmentManager = getFragmentManager(); | ||
| PeopleListFragment peopleListFragment = (PeopleListFragment) fragmentManager | ||
| .findFragmentByTag(KEY_PEOPLE_LIST_FRAGMENT); | ||
| PersonDetailFragment personDetailFragment = (PersonDetailFragment) fragmentManager | ||
| .findFragmentByTag(KEY_PERSON_DETAIL_FRAGMENT); | ||
|
|
||
| if (peopleListFragment != null) { | ||
| peopleListFragment.refreshPeopleList(); | ||
| } | ||
| if (personDetailFragment != null) { | ||
| personDetailFragment.refreshPersonDetails(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -97,4 +107,25 @@ public void onJSONException(JSONException exception) { | |
| } | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public void onPersonSelected(Person person) { | ||
| FragmentManager fragmentManager = getFragmentManager(); | ||
| PersonDetailFragment personDetailFragment = (PersonDetailFragment) fragmentManager | ||
| .findFragmentByTag(KEY_PERSON_DETAIL_FRAGMENT); | ||
|
|
||
| long personID = person.getPersonID(); | ||
| int localTableBlogID = person.getLocalTableBlogId(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Earlier in the file, a semantically same variable/parameter is named |
||
| if (personDetailFragment == null) { | ||
| personDetailFragment = PersonDetailFragment.newInstance(personID, localTableBlogID); | ||
| } else { | ||
| personDetailFragment.setPersonDetails(personID, localTableBlogID); | ||
| } | ||
| if (!personDetailFragment.isAdded()) { | ||
| FragmentTransaction fragmentTransaction = getFragmentManager().beginTransaction(); | ||
| fragmentTransaction.add(R.id.fragment_container, personDetailFragment, KEY_PERSON_DETAIL_FRAGMENT); | ||
| fragmentTransaction.addToBackStack(null); | ||
| fragmentTransaction.commit(); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For consistency, let's define these constants with all-caps strings, like earlier in this PR (https://github.com/wordpress-mobile/WordPress-Android/pull/3970/files#diff-76558a95ecd9db2c0dda264a760a7e48R20). Cheers!
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.
You're right those 2 are different, but maybe I should have used all-lowercase on the other one looking at all the other files:
https://github.com/wordpress-mobile/WordPress-Android/blob/develop/WordPress/src/main/java/org/wordpress/android/ui/prefs/AccountSettingsActivity.java#L12
https://github.com/wordpress-mobile/WordPress-Android/blob/develop/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/EditorFragment.java#L61-L73
https://github.com/wordpress-mobile/WordPress-Android/blob/develop/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsActivity.java#L14-L15
https://github.com/wordpress-mobile/WordPress-Android/blob/develop/WordPress/src/main/java/org/wordpress/android/ui/comments/CommentsActivity.java#L38
Looking at the style guidelines we use I don't see any mention of the right side expression of constant declarations. So, maybe let's go with the rest of the app and use all lowercase instead. Sounds good?
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.
All-lowercase is fine by me, let's go with that +1