-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Let devs intercept onChanged events and control when to update their adapter #488
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
samtstern
merged 29 commits into
firebase:version-1.2.0-dev
from
SUPERCILEX:enable-filtering+predicates
Jan 24, 2017
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
5d5635a
Update api to let users do their own filtering + sorting
SUPERCILEX 0e3ebbc
Cleanup
SUPERCILEX 9d462dc
Cleanup
SUPERCILEX 276a5ba
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX 0b01f48
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX 9ea660e
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX 0f12fe3
Add documentation
SUPERCILEX 0fa222d
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX 2899205
Cleanup
SUPERCILEX 0826541
Cleanup
SUPERCILEX bc43a01
Merge remote-tracking branch 'origin/enable-filtering+predicates' int…
SUPERCILEX e8a9905
Merge branch 'version-1.1.0-dev' into enable-filtering+predicates
SUPERCILEX ab3743e
Extract onComplete listener into static inner class to prevent activi…
SUPERCILEX 08159bd
Cleanup
SUPERCILEX b066b54
Temporarily ignore robolectric updates
SUPERCILEX 3e5b71d
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX 1c1dfb7
Address review comments
SUPERCILEX a9bad96
Rename `OnChangedListener` to `ChangeEventListener` and `onChanged` t…
SUPERCILEX f875b48
Rename onChildChanged in adapters
SUPERCILEX 142a015
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into enable…
SUPERCILEX c85ca14
Fix merge mistakes
SUPERCILEX 3a46033
Merge remote-tracking branch 'firebase/master' into enable-filtering+…
SUPERCILEX 50d3d37
Fix merge mistakes
SUPERCILEX 17a136d
Fix more merge mistakes
SUPERCILEX 4b74478
Merge remote-tracking branch 'firebase/master' into enable-filtering+…
SUPERCILEX 358d50d
Cleanup
SUPERCILEX 4496baf
Cleanup
SUPERCILEX dcf4b91
Merge remote-tracking branch 'origin/enable-filtering+predicates' int…
SUPERCILEX 499d448
Merge remote-tracking branch 'firebase/version-1.2.0-dev' into enable…
SUPERCILEX 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
|
||
package com.firebase.uidemo.database; | ||
|
||
import android.content.Context; | ||
import android.graphics.PorterDuff; | ||
import android.graphics.drawable.GradientDrawable; | ||
import android.graphics.drawable.RotateDrawable; | ||
|
@@ -37,6 +38,7 @@ | |
import com.firebase.ui.database.FirebaseRecyclerAdapter; | ||
import com.firebase.uidemo.R; | ||
import com.google.android.gms.tasks.OnCompleteListener; | ||
import com.google.android.gms.tasks.OnSuccessListener; | ||
import com.google.android.gms.tasks.Task; | ||
import com.google.firebase.auth.AuthResult; | ||
import com.google.firebase.auth.FirebaseAuth; | ||
|
@@ -86,9 +88,9 @@ public void onClick(View v) { | |
Chat chat = new Chat(name, mMessageEdit.getText().toString(), uid); | ||
mChatRef.push().setValue(chat, new DatabaseReference.CompletionListener() { | ||
@Override | ||
public void onComplete(DatabaseError databaseError, DatabaseReference reference) { | ||
if (databaseError != null) { | ||
Log.e(TAG, "Failed to write message", databaseError.toException()); | ||
public void onComplete(DatabaseError error, DatabaseReference reference) { | ||
if (error != null) { | ||
Log.e(TAG, "Failed to write message", error.toException()); | ||
} | ||
} | ||
}); | ||
|
@@ -97,11 +99,10 @@ public void onComplete(DatabaseError databaseError, DatabaseReference reference) | |
} | ||
}); | ||
|
||
mMessages = (RecyclerView) findViewById(R.id.messagesList); | ||
|
||
mManager = new LinearLayoutManager(this); | ||
mManager.setReverseLayout(false); | ||
|
||
mMessages = (RecyclerView) findViewById(R.id.messagesList); | ||
mMessages.setHasFixedSize(false); | ||
mMessages.setLayoutManager(mManager); | ||
} | ||
|
@@ -145,7 +146,6 @@ private void attachRecyclerViewAdapter() { | |
Query lastFifty = mChatRef.limitToLast(50); | ||
mRecyclerViewAdapter = new FirebaseRecyclerAdapter<Chat, ChatHolder>( | ||
Chat.class, R.layout.message, ChatHolder.class, lastFifty) { | ||
|
||
@Override | ||
public void populateViewHolder(ChatHolder chatView, Chat chat, int position) { | ||
chatView.setName(chat.getName()); | ||
|
@@ -180,20 +180,13 @@ public void onItemRangeInserted(int positionStart, int itemCount) { | |
private void signInAnonymously() { | ||
Toast.makeText(this, "Signing in...", Toast.LENGTH_SHORT).show(); | ||
mAuth.signInAnonymously() | ||
.addOnCompleteListener(this, new OnCompleteListener<AuthResult>() { | ||
.addOnSuccessListener(this, new OnSuccessListener<AuthResult>() { | ||
@Override | ||
public void onComplete(@NonNull Task<AuthResult> task) { | ||
Log.d(TAG, "signInAnonymously:onComplete:" + task.isSuccessful()); | ||
if (task.isSuccessful()) { | ||
Toast.makeText(ChatActivity.this, "Signed In", | ||
Toast.LENGTH_SHORT).show(); | ||
attachRecyclerViewAdapter(); | ||
} else { | ||
Toast.makeText(ChatActivity.this, "Sign In Failed", | ||
Toast.LENGTH_SHORT).show(); | ||
} | ||
public void onSuccess(AuthResult result) { | ||
attachRecyclerViewAdapter(); | ||
} | ||
}); | ||
}) | ||
.addOnCompleteListener(new SignInResultNotifier(this)); | ||
} | ||
|
||
public boolean isSignedIn() { | ||
|
@@ -297,4 +290,24 @@ public void setText(String text) { | |
mTextField.setText(text); | ||
} | ||
} | ||
|
||
/** | ||
* Notifies the user of sign in successes or failures beyond the lifecycle of an activity. | ||
*/ | ||
private static class SignInResultNotifier implements OnCompleteListener<AuthResult> { | ||
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. Nice addition! I'd appreciate some documentation on this class though. |
||
private Context mContext; | ||
|
||
public SignInResultNotifier(Context context) { | ||
mContext = context.getApplicationContext(); | ||
} | ||
|
||
@Override | ||
public void onComplete(@NonNull Task<AuthResult> task) { | ||
if (task.isSuccessful()) { | ||
Toast.makeText(mContext, R.string.signed_in, Toast.LENGTH_SHORT).show(); | ||
} else { | ||
Toast.makeText(mContext, R.string.sign_in_failed, Toast.LENGTH_SHORT).show(); | ||
} | ||
} | ||
} | ||
} |
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
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
65 changes: 65 additions & 0 deletions
65
database/src/main/java/com/firebase/ui/database/ChangeEventListener.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,65 @@ | ||
package com.firebase.ui.database; | ||
|
||
import com.google.firebase.database.ChildEventListener; | ||
import com.google.firebase.database.DataSnapshot; | ||
import com.google.firebase.database.DatabaseError; | ||
|
||
public interface ChangeEventListener { | ||
/** | ||
* The type of event received when a child has been updated. | ||
*/ | ||
enum EventType { | ||
/** | ||
* An onChildAdded event was received. | ||
* | ||
* @see ChildEventListener#onChildAdded(DataSnapshot, String) | ||
*/ | ||
ADDED, | ||
/** | ||
* An onChildChanged event was received. | ||
* | ||
* @see ChildEventListener#onChildChanged(DataSnapshot, String) | ||
*/ | ||
CHANGED, | ||
/** | ||
* An onChildRemoved event was received. | ||
* | ||
* @see ChildEventListener#onChildRemoved(DataSnapshot) | ||
*/ | ||
REMOVED, | ||
/** | ||
* An onChildMoved event was received. | ||
* | ||
* @see ChildEventListener#onChildMoved(DataSnapshot, String) | ||
*/ | ||
MOVED | ||
} | ||
|
||
/** | ||
* A callback for when a child has changed in FirebaseArray. | ||
* | ||
* @param type The type of event received | ||
* @param index The index at which the change occurred | ||
* @param oldIndex If {@code type} is a moved event, the previous index of the moved child. | ||
* For any other event, {@code oldIndex} will be -1. | ||
*/ | ||
void onChildChanged(EventType type, int index, int oldIndex); | ||
|
||
/** This method will be triggered each time updates from the database have been completely processed. | ||
* So the first time this method is called, the initial data has been loaded - including the case | ||
* when no data at all is available. Each next time the method is called, a complete update (potentially | ||
* consisting of updates to multiple child items) has been completed. | ||
* <p> | ||
* You would typically override this method to hide a loading indicator (after the initial load) or | ||
* to complete a batch update to a UI element. | ||
*/ | ||
void onDataChanged(); | ||
|
||
/** | ||
* This method will be triggered in the event that this listener either failed at the server, | ||
* or is removed as a result of the security and Firebase Database rules. | ||
* | ||
* @param error A description of the error that occurred | ||
*/ | ||
void onCancelled(DatabaseError error); | ||
} |
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 |
---|---|---|
|
@@ -27,19 +27,8 @@ | |
* This class implements an array-like collection on top of a Firebase location. | ||
*/ | ||
class FirebaseArray implements ChildEventListener, ValueEventListener { | ||
public interface OnChangedListener { | ||
enum EventType {ADDED, CHANGED, REMOVED, MOVED} | ||
|
||
void onChildChanged(EventType type, int index, int oldIndex); | ||
|
||
void onDataChanged(); | ||
|
||
void onCancelled(DatabaseError databaseError); | ||
|
||
} | ||
|
||
private Query mQuery; | ||
private OnChangedListener mListener; | ||
private ChangeEventListener mListener; | ||
private List<DataSnapshot> mSnapshots = new ArrayList<>(); | ||
|
||
public FirebaseArray(Query ref) { | ||
|
@@ -80,21 +69,21 @@ public void onChildAdded(DataSnapshot snapshot, String previousChildKey) { | |
index = getIndexForKey(previousChildKey) + 1; | ||
} | ||
mSnapshots.add(index, snapshot); | ||
notifyChangedListeners(OnChangedListener.EventType.ADDED, index); | ||
notifyChangedListeners(ChangeEventListener.EventType.ADDED, index); | ||
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. Definitely a better name! +1 |
||
} | ||
|
||
@Override | ||
public void onChildChanged(DataSnapshot snapshot, String previousChildKey) { | ||
int index = getIndexForKey(snapshot.getKey()); | ||
mSnapshots.set(index, snapshot); | ||
notifyChangedListeners(OnChangedListener.EventType.CHANGED, index); | ||
notifyChangedListeners(ChangeEventListener.EventType.CHANGED, index); | ||
} | ||
|
||
@Override | ||
public void onChildRemoved(DataSnapshot snapshot) { | ||
int index = getIndexForKey(snapshot.getKey()); | ||
mSnapshots.remove(index); | ||
notifyChangedListeners(OnChangedListener.EventType.REMOVED, index); | ||
notifyChangedListeners(ChangeEventListener.EventType.REMOVED, index); | ||
} | ||
|
||
@Override | ||
|
@@ -103,7 +92,7 @@ public void onChildMoved(DataSnapshot snapshot, String previousChildKey) { | |
mSnapshots.remove(oldIndex); | ||
int newIndex = previousChildKey == null ? 0 : (getIndexForKey(previousChildKey) + 1); | ||
mSnapshots.add(newIndex, snapshot); | ||
notifyChangedListeners(OnChangedListener.EventType.MOVED, newIndex, oldIndex); | ||
notifyChangedListeners(ChangeEventListener.EventType.MOVED, newIndex, oldIndex); | ||
} | ||
|
||
@Override | ||
|
@@ -116,23 +105,23 @@ public void onCancelled(DatabaseError error) { | |
notifyCancelledListeners(error); | ||
} | ||
|
||
public void setOnChangedListener(OnChangedListener listener) { | ||
public void setOnChangedListener(ChangeEventListener listener) { | ||
mListener = listener; | ||
} | ||
|
||
protected void notifyChangedListeners(OnChangedListener.EventType type, int index) { | ||
protected void notifyChangedListeners(ChangeEventListener.EventType type, int index) { | ||
notifyChangedListeners(type, index, -1); | ||
} | ||
|
||
protected void notifyChangedListeners(OnChangedListener.EventType type, int index, int oldIndex) { | ||
protected void notifyChangedListeners(ChangeEventListener.EventType type, int index, int oldIndex) { | ||
if (mListener != null) { | ||
mListener.onChildChanged(type, index, oldIndex); | ||
} | ||
} | ||
|
||
protected void notifyCancelledListeners(DatabaseError databaseError) { | ||
protected void notifyCancelledListeners(DatabaseError error) { | ||
if (mListener != null) { | ||
mListener.onCancelled(databaseError); | ||
mListener.onCancelled(error); | ||
} | ||
} | ||
} |
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
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.
Unnecessary, these are defaults.
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.
There are valid reasons for having this code here, which we could debate here. But these reasons have nothing to do with letting devs intercept change events. In future PRs please keep functional changes separate from code cleanup.
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 removed this change, but I am curious, why are we setting default values and adding seemingly unnecessary code?