feat(docs): Firebase Storage docs #902

davideast · 2017-01-17T15:32:48Z

Documentation for the $firebaseStorage() service and firebase-src directive.

coveralls · 2017-01-17T15:35:28Z

Coverage remained the same at 93.716% when pulling d4fc74f on storage-docs into 94ca5bc on master.

jwngr

Need to go run to a meeting, but I'm about two third of the way done and figured I'd send you what I have so far. Overall, the content looks good. Mainly just have minor suggestions around styling and formatting.

jwngr · 2017-01-17T17:44:34Z

docs/guide/uploading-downloading-binary-content.md

+
+* [Overview](#overview)
+* [API Summary](#api-summary)
+* [Uploading files](#uploading-files)


All other guide pages use all-caps for the section titles. Please update these to make them consistent.

jwngr · 2017-01-17T17:45:03Z

docs/guide/uploading-downloading-binary-content.md

+## Overview
+
+Firebase provides [a hosted binary storage service](https://firebase.google.com/docs/storage/)
+which enables you to store and retrieve user-generated content like images, audio and


Nit: add Oxford comma after audio since we use it elsewhere.

jwngr · 2017-01-17T17:45:20Z

docs/guide/uploading-downloading-binary-content.md

+video directly from the Firebase client SDK.
+
+Binary files are stored in a Firebase Storage bucket, not in the Realtime Database.
+The file files in your bucket are stored in a hierarchical structure, just like


s/file files/files

jwngr · 2017-01-17T17:45:58Z

docs/guide/uploading-downloading-binary-content.md

+in the Realtime Database.
+
+To use the Firebase Storage binding, first [create a Firebase Storage reference](https://firebase.google.com/docs/storage/web/create-reference).
+Then using this reference, pass it into the `$firebaseStorage` service.


s/Then using/Then, using

Also, replace the closing period with a colon.

jwngr · 2017-01-17T17:46:38Z

docs/guide/uploading-downloading-binary-content.md

+// inject $firebaseStorage into our controller
+function SampleCtrl($firebaseStorage) {
+  // create a Firebase Storage Reference for the $firebaseStorage binding
+  var storageRef = firebase.storage().ref('userProfiles/physicsmarie');


Use double quotes.

jwngr · 2017-01-17T17:54:49Z

docs/guide/uploading-downloading-binary-content.md

+  var storageRef = firebase.storage().ref('userProfiles/physicsmarie');
+  var storage = $firebaseStorage(storageRef);
+}
+SampleCtrl.$inject = ["$firebaseStorage"];


Do we actually need this line? Also, can we make this style consistent with how we define the module and controller elsewhere in the docs (e.g. here)?

jwngr · 2017-01-17T17:56:28Z

docs/guide/uploading-downloading-binary-content.md

+
+| Method  | Description |
+| ------------- | ------------- |
+| [`$progress(callback)`](/docs/reference.md#uploadtask-progress) |	Calls the provided callback function whenever there is an update in the progress of the file uploading. |


Links for all of these methods are broken. It looks like they don't need a leading "uploadtask-".

jwngr · 2017-01-17T17:56:49Z

docs/guide/uploading-downloading-binary-content.md

+| Method  | Description |
+| ------------- | ------------- |
+| [`$progress(callback)`](/docs/reference.md#uploadtask-progress) |	Calls the provided callback function whenever there is an update in the progress of the file uploading. |
+| [`$error(callback)`](/docs/reference.md#uploadtask-error)	| Calls the provided callback function when there is an error uploading the file |


Add period to end of description.

jwngr · 2017-01-17T17:58:04Z

docs/guide/uploading-downloading-binary-content.md

+| [`$cancel()`](/docs/reference.md#uploadtask-cancel) | Cancels the upload. |
+| [`$pause()`](/docs/reference.md#uploadtask-pause) | Pauses the upload. |
+| [`$snapshot()`](/docs/reference.md#uploadtask-$snapshot) | Returns the [current immutable view of the task](https://firebase.google.com/docs/storage/web/upload-files#monitor_upload_progress) at the time the event occurred. |
+| [`then(callback)`](/docs/reference.md#uploadtask-then) | An Upload Task implements a promise like interface. The callback is called when the upload is complete. |


s/Upload Task/UploadTask (and add backticks around it)
s/promise/Promise (and add backticks around it)
s/The callback/This callback

jwngr · 2017-01-17T17:58:14Z

docs/guide/uploading-downloading-binary-content.md

+| [`$pause()`](/docs/reference.md#uploadtask-pause) | Pauses the upload. |
+| [`$snapshot()`](/docs/reference.md#uploadtask-$snapshot) | Returns the [current immutable view of the task](https://firebase.google.com/docs/storage/web/upload-files#monitor_upload_progress) at the time the event occurred. |
+| [`then(callback)`](/docs/reference.md#uploadtask-then) | An Upload Task implements a promise like interface. The callback is called when the upload is complete. |
+| [`catch(callback)`](/docs/reference.md#uploadtask-then) | An Upload Task implements a promise like interface. The callback is called when an error occurs. |


Ditto on the comment above.

jwngr

Okay, all done with my first pass.

jwngr · 2017-01-17T18:00:39Z

docs/guide/uploading-downloading-binary-content.md

+| [`catch(callback)`](/docs/reference.md#uploadtask-then) | An Upload Task implements a promise like interface. The callback is called when an error occurs. |
+
+## Displaying images with the `firebase-src` directive
+AngularFire provides a directive that displays a file with any `src` compatible element.


Add a new line after the header.

Suggestion: src-compatible

jwngr · 2017-01-17T18:01:43Z

docs/guide/uploading-downloading-binary-content.md

+## Displaying images with the `firebase-src` directive
+AngularFire provides a directive that displays a file with any `src` compatible element.
+
+Instead of using the tradional `src` attribute, use `firebase-src`:


Can we just make this part of the previous paragraph?

jwngr · 2017-01-17T18:02:15Z

docs/guide/uploading-downloading-binary-content.md

+```
+
+## Retrieving files from the template
+AngularFire does not provide a directive for retrieving a upload file. However,


Add new line after header.

s/a upload/an uploaded

jwngr · 2017-01-17T18:03:03Z

docs/guide/uploading-downloading-binary-content.md

+
+## Retrieving files from the template
+AngularFire does not provide a directive for retrieving a upload file. However,
+the below directive is basic but provides a baseline to work off.


Suggestion: get rid of "is basic but" and replace the closing period with a colon.

jwngr · 2017-01-17T18:04:03Z

docs/guide/uploading-downloading-binary-content.md

+    scope: {
+      onChange: "="
+    },
+    template: "<input type="file" name="file" class="inputfile" /><label><ng-transclude></ng-transclude></label>",


For simplicity, I would suggest dropping the class attribute as it isn't really required for this example to work.

Within the template, you should use single quotes or escape the double quotes.

jwngr · 2017-01-17T18:23:58Z

docs/reference.md

+var asString = $scope.storage.$toString();
+```
+
+## Upload Task


This should have three # characters.

jwngr · 2017-01-17T18:24:14Z

docs/reference.md

+var uploadTask = $scope.storage.$put(htmlFile, { contentType: 'text/html' });
+```
+
+## $putString(string, format, metadata)


All methods directly off of $firebaseStorage should have three # characters, not two.

jwngr · 2017-01-17T18:24:41Z

docs/reference.md

+var uploadTask = $scope.storage.$put(htmlFile, { contentType: 'text/html' });
+```
+
+## $progress(callback)


All headers under Upload Task should have four # characters, not two.

jwngr · 2017-01-17T18:24:56Z

docs/reference.md

+$scope.bytesTransferred = uploadTask.$snapshot.bytesTransferred;
+```
+
+## Upload Task then()


Can this just be then()?

jwngr · 2017-01-17T18:25:18Z

docs/reference.md

+});
+```
+
+## Upload Task catch()


Can this just be catch()?

CyrusZei · 2017-01-18T08:57:10Z

Oh this is perfect, You guys are soon done with it and I just cant wait! Keep up the good work guys, you are the best

coveralls · 2017-01-18T16:44:16Z

Coverage remained the same at 93.716% when pulling 7c990e7 on storage-docs into 94ca5bc on master.

coveralls · 2017-01-18T17:46:16Z

Coverage remained the same at 93.716% when pulling 8b5244b on storage-docs into 94ca5bc on master.

…into storage-docs

coveralls · 2017-01-18T17:50:24Z

Coverage remained the same at 93.716% when pulling 5cfa40e on storage-docs into 94ca5bc on master.

coveralls · 2017-01-18T18:09:09Z

Coverage remained the same at 93.716% when pulling b34edba on storage-docs into 94ca5bc on master.

jwngr

I think we are good after a few last edits.

jwngr · 2017-01-19T01:12:34Z

docs/guide/uploading-downloading-binary-content.md

+| [`then(callback)`](/docs/reference.md#then) | An `UploadTask` implements a `Promise` like interface. This callback is called when the upload is complete. |
+| [`catch(callback)`](/docs/reference.md#catch) | An `UploadTask` implements a `Promise` like interface. This callback is called when an error occurs. |
+
+## Displaying images with the `firebase-src` directive


Should be title-case.

jwngr · 2017-01-19T01:12:38Z

docs/guide/uploading-downloading-binary-content.md

+<img firebase-src="{{ userProfileId }}" />
+```
+
+## Retrieving files from the template


Should be title-case.

jwngr · 2017-01-19T01:15:14Z

docs/guide/uploading-downloading-binary-content.md

@@ -0,0 +1,147 @@
+# Uploading & Downloading Binary Content | AngularFire Guide


A link to this page needs to get added to https://github.com/firebase/angularfire/blob/master/docs/guide/README.md. I think putting it as #4 (in between the Database and Auth sections) makes the most sense. Also, we need to make sure to update the link at the end of guide section #3 now links to this guide and, at the end of this guide, link to guide section #5 (which will now be the Auth section).

jwngr · 2017-01-19T01:33:01Z

docs/guide/uploading-downloading-binary-content.md

+
+| Method  | Description |
+| ------------- | ------------- |
+| [`$put(file, metadata)`](/docs/reference.md#putfile-metadata) |	Uploads file to configured path with optional metadata. Returns an AngularFire wrapped `UploadTask`. |


Let's link UploadTask to #upload-task-api-summary everywhere in this API summary (four instances).

jwngr · 2017-01-19T01:34:28Z

docs/guide/uploading-downloading-binary-content.md

+
+## Retrieving files from the template
+
+AngularFire does not provide a directive for retrieving an upload file. However,


s/upload/uploaded

jwngr · 2017-01-19T01:41:41Z

docs/reference.md

+Returns a promise fulfilled with the updated metadata.
+
+```js
+var updateData = { contenType: 'text/plain' };


Use double quotes.

jwngr · 2017-01-19T01:42:02Z

docs/reference.md

+
+```js
+var updateData = { contenType: 'text/plain' };
+$scope.storage.$updateMetadata(updateData).then(function(completeMetadata) {


s/completeMetadata/updatedMetadata

jwngr · 2017-01-19T01:42:18Z

docs/reference.md

+
+```js
+$scope.storage.$delete().then(function() {
+  console.log('successfully deleted!');


Use double quotes.

jwngr · 2017-01-19T01:43:22Z

docs/reference.md

+### Upload Task
+
+The [`$firebaseStorage()`](#firebasestorage) service returns an AngularFire wrapped [`UploadTask`](https://firebase.google.com/docs/reference/js/firebase.storage#uploadtask) when uploading binary content
+using the [`$put()`]($putfile-metadata) and [`$putString()`](#putstringstring-format-metadata) methods. This task is used for [monitoring](https://firebase.google.com/docs/storage/web/upload-files#monitor_upload_progress)


The link for $put() 404s.

jwngr · 2017-01-19T01:43:46Z

docs/reference.md

+and [managing](https://firebase.google.com/docs/storage/web/upload-files#manage_uploads) uploads.
+
+```js
+var htmlFile = new Blob(["<html></html>"], {type : "text/html"});


Ditto on comment above regarding padding spaces.

coveralls · 2017-01-23T17:31:59Z

Coverage remained the same at 93.716% when pulling 82c3cf1 on storage-docs into 94ca5bc on master.

coveralls · 2017-01-23T17:41:47Z

Coverage remained the same at 93.716% when pulling 73f2e77 on storage-docs into 94ca5bc on master.

davideast added 4 commits January 17, 2017 06:05

feat(docs): Initial draft of Firebase Storage docs

a2ba624

feat(docs): () API Reference

940efd6

feat(docs): UploadTask API Reference

2ba69f8

fix(docs): Fix # links in the docs

d4fc74f

davideast requested a review from jwngr January 17, 2017 15:33

davideast assigned jwngr Jan 17, 2017

jwngr suggested changes Jan 17, 2017

View reviewed changes

chore(docs): Jacob's comments

7c990e7

chore(docs): Jacob's comments

8b5244b

davideast added 2 commits January 18, 2017 09:46

chore(docs): Jacob's comments

de778ef

Merge branch 'storage-docs' of https://github.com/firebase/angularfire …

5cfa40e

…into storage-docs

chore(docs): Jacob's comments

b34edba

jwngr suggested changes Jan 19, 2017

View reviewed changes

jwngr assigned davideast and unassigned jwngr Jan 19, 2017

chore(docs): Jacob's comments

82c3cf1

chore(docs): Make it on one line

73f2e77

jwngr approved these changes Jan 23, 2017

View reviewed changes

davideast merged commit fe25154 into master Jan 23, 2017

davideast deleted the storage-docs branch January 23, 2017 17:44

		@@ -0,0 +1,147 @@
		# Uploading & Downloading Binary Content \| AngularFire Guide


		## Retrieving files from the template

		AngularFire does not provide a directive for retrieving an upload file. However,

feat(docs): Firebase Storage docs #902

feat(docs): Firebase Storage docs #902

Conversation

davideast commented Jan 17, 2017

coveralls commented Jan 17, 2017 • edited

jwngr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwngr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusZei commented Jan 18, 2017

coveralls commented Jan 18, 2017

coveralls commented Jan 18, 2017

coveralls commented Jan 18, 2017

coveralls commented Jan 18, 2017 • edited

jwngr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 23, 2017

coveralls commented Jan 23, 2017

coveralls commented Jan 17, 2017 •

edited

coveralls commented Jan 18, 2017 •

edited