Skip to content

Version 0.6.0, FirebaseUI Storage #324

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
merged 2 commits into from
Sep 24, 2016
Merged

Conversation

samtstern
Copy link
Contributor

Implements FirebaseImageLoader and bumps overall version to 0.6.0.

@samtstern
Copy link
Contributor Author

cc: @mcdonamp @morganchen12

Not sure who should review this, likely @puf

Copy link
Contributor

@asciimike asciimike left a comment

Choose a reason for hiding this comment

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

A few general comments, but otherwise LGTM.

setContentView(R.layout.activity_image);
ButterKnife.bind(this);

// Authenticate so that uploading/downloading works
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the other samples work or do we just set rules? I'd consider making auth global (across all sample features--though I guess it doesn't make sense for auth ;), or just getting devs to change their rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since authenticated downloads is a feature of the lib I'd like to keep auth, but I will add a comment here to explain and log some better resolution steps.

if (!task.isSuccessful()) {
Log.w(TAG, "signInAnonymously", task.getException());
Toast.makeText(ImageActivity.this,
"Authentication failed, uploads and downloads will not work.",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should include a resolution step here: enable auth or change your rules (or something strange happened)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added log message with detailed instructions.


## Using FirebaseUI to download and display images

FirebaseUI provides bindings to download an image file from a `StorageReference` and display it
Copy link
Contributor

Choose a reason for hiding this comment

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

Include links to StorageReference and Glide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might also mention what Storage is with a brief blurb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


FirebaseUI provides bindings to download an image file from a `StorageReference` and display it
using the popular `Glide` library. This technique allows you to get all of Glide's performance
benefits while leveraging Firebase Storage's authenticated hosting capabilities.
Copy link
Contributor

Choose a reason for hiding this comment

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

"authenticated storage..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.into(imageView);
```

Images displayed using `FirebaseImageLoader` are cached by their path in Firebase Storage, so
Copy link
Contributor

Choose a reason for hiding this comment

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

Disk or memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whatever the user sets. Adding link.


compile 'com.github.bumptech.glide:glide:3.7.0'

compile "com.google.firebase:firebase-auth:${project.ext.firebase_version}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it actually needs auth, right? It can work without authentication if rules are set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch.

@Override
public InputStream loadData(Priority priority) throws Exception {
mStreamTask = mRef.getStream();
mInputStream = Tasks.await(mStreamTask).getStream();
Copy link
Contributor

Choose a reason for hiding this comment

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

how are errors propagated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I call .await() that will throw an Exception if something goes wrong. That will be handled by Glide, and the user can attach failure listeners to Glide if they want to know about this.

@samtstern samtstern merged commit 3ee51f2 into firebase:master Sep 24, 2016
amandle pushed a commit to amandle/FirebaseUI-Android that referenced this pull request Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants