Add Community Supported tvOS #590

paulb777 · 2017-12-21T23:45:58Z

Add Community Supported tvOS for Core, Auth, Database and Storage.
Add tvOS unit tests
Add tvOS sample app
Update README.md
Add tvOS to travis testing

Initial tvOS project sample.

* Core Unit Tests for tvOS * tvOS 10 and shared schemes * copyrights * style fixes

* Example using Firebase Storage. * Remove unnecessary AppDelegate methods * Fixed comment on constants

* Build and unit test Database for tvOS * delete blank line

* Build environment for tvOS Auth * Added Auth example for email and password sign in. * Fixed TARGET_OS_TV check * Cleaned up storyboard

Merge master to tvOS master

ryanwilson

Exciting! It'll be great to get Firestore support too if possible. LGTM, will wait to see what the others think.

schmidt-sebastian

This looks awesome! I have some question about the Storage and Database sample.

schmidt-sebastian · 2017-12-22T19:03:03Z

Example/tvOSSample/tvOSSample/DatabaseViewController.swift

+import UIKit
+import FirebaseDatabase
+
+/// A class to demonstrate the power of the Firebase Realtime Database. This will show a number read


Nit: would prefer if this just read "A class to demonstrate the Firebase Realtime Database API."

schmidt-sebastian · 2017-12-22T19:03:22Z

Example/tvOSSample/tvOSSample/DatabaseViewController.swift

+  // MARK: - Internal Helpers
+
+  /// Update the number on the server by a particular value. Note: the number passed in should only
+  /// be


Please fix this comment.

schmidt-sebastian · 2017-12-22T19:03:33Z

Example/tvOSSample/tvOSSample/DatabaseViewController.swift

+  /// be
+  private func changeServerValue(with type: Counter) {
+    let ref = Database.database().reference(withPath: Constants.databasePath)
+    // Update the current value of the number!


Nit: Remove "!"

schmidt-sebastian · 2017-12-22T19:04:39Z

Example/tvOSSample/tvOSSample/DatabaseViewController.swift

+        return TransactionResult.abort()
+      }
+
+      currentData.value = value + type.rawValue


I don't write much Swift, but it seems a little strange to me to use the index value of an enum in a computation.

I've used enums a bit like this since they're a lot more flexible and not limited to just a raw value anymore. It could be expanded in the future to have different values, it just saves the switch statement.

I added an intValue calculated property that returns the rawValue internally, so it's more of an implementation detail as opposed to relying on the functionality externally. Does that make more sense?

Yes, it does. Thanks for cleaning this up.

schmidt-sebastian · 2017-12-22T19:06:31Z

Example/tvOSSample/tvOSSample/StorageViewController.swift

+
+    // We could have the image cached already causing the completion block to run before this line.
+    // If that's the case, we don't need to do anything else and can return.
+    if case .downloaded = state { return }


Firebase Storage doesn't have a cache. Can you explain what is going on here? Thanks!

Huh, interesting. There were a few situations where if you already downloaded the photo + cleared it, the completion block setting the state to downloaded would be triggered before the code afterward setting it to downloading. Maybe this is Apple's caching under the hood?

It could be using an NSURLCache by default, but even with that I would be surprised that this code would race. There might be something else going on here, but I am not quite sure what it could be.
Either way, please remove the word "cache" from the comment so that our users don't get any wrong ideas :)

Updated, thanks!

schmidt-sebastian · 2017-12-22T19:08:59Z

Example/tvOSSample/tvOSSample/StorageViewController.swift

+    // Start the download!
+    let storage = Storage.storage()
+    let ref = storage.reference(withPath: Constants.downloadPath)
+    let task = ref.getData(maxSize: Constants.maxSize) { [unowned self] (data, error) in


Should we showcase how to display progress updates as well?

schmidt-sebastian · 2018-01-02T10:12:06Z

Example/tvOSSample/tvOSSample/StorageViewController.swift

+
+    // We could have the image cached already causing the completion block to run before this line.
+    // If that's the case, we don't need to do anything else and can return.
+    if case .downloaded = state { return }


It could be using an NSURLCache by default, but even with that I would be surprised that this code would race. There might be something else going on here, but I am not quite sure what it could be.
Either way, please remove the word "cache" from the comment so that our users don't get any wrong ideas :)

protocol86 · 2018-01-02T18:51:08Z

Firebase/Auth/Source/FIRAuthAPNSTokenManager.m

@@ -73,7 +73,9 @@ - (void)getTokenWithCallback:(FIRAuthAPNSTokenCallback)callback {
    } else {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#if !TARGET_OS_TV


I'd prefer using #if TARGET_OS_IOS here as this should only be used for the iOS platform.
I also suspect that this check might not be needed as an APNS token manager should only instantiated on the Auth instance for iOS targets. Did you find this check necessary?

Fixed! The check is necessary since UIApplication doesn't have a registerForRemoteNotificationTypes: method available in tvOS so this fails to compile unless it's wrapped.

Gotcha :) thanks for the background.

protocol86 · 2018-01-02T18:57:34Z

Example/tvOSSample/tvOSSample/EmailLoginViewController.swift

+
+    Auth.auth().signIn(withEmail: email, password: password) { [unowned self] (user, error) in
+      guard let user = user else {
+        print("Error retrieving user: \(error!)")


"Error signing in" seems more fitting here.

Makes sense, done.

protocol86 · 2018-01-02T19:04:11Z

Example/tvOSSample/tvOSSample/EmailLoginViewController.swift

+      return nil
+    }
+
+    guard let userPassword = password.text, userPassword.count > 2 else {


Firebase Auth backend currently enforces a minimum password length of 6 characters.

Fixed, thanks!

protocol86 · 2018-01-02T19:05:20Z

Example/tvOSSample/tvOSSample/EmailLoginViewController.swift

+  /// Validate the inputs for user email and password, returning the username and password if valid,
+  /// otherwise nil.
+  private func validatedInputs() -> (email: String, password: String)? {
+    guard let userEmail = emailAddress.text, userEmail.count > 3 else {


The Firebase Auth backend does email validation, you may want to rely on the backend validation in this case.

Ah okay, I figured this way if someone just tries to hit "Go" without typing anything in, we'd throw up an error before bothering the backend. In a real scenario I'd probably highlight this in the UI, etc but this was a quick fix. I can remove it entirely but I don't think it hurts having it here, since it's more of a pre-validation than a full validation.

Sounds good. We refrain from doing validation on the client because it's easier and faster to update the server if needs be. Makes total senses to save the round trip in a sample app.

protocol86 · 2018-01-02T19:06:15Z

Example/tvOSSample/tvOSSample/EmailLoginViewController.swift

+
+    Auth.auth().createUser(withEmail: email, password: password) { [unowned self] (user, error) in
+      guard let user = user else {
+        print("Error retrieving user: \(error!)")


"Error signing up" seems more fitting here.

Nice catch! Done

mariohahn · 2018-01-04T08:45:47Z

😍

* Update CHANGELOG for macOS AppKit dependency. (#609) * Add Community Supported tvOS (#590) Add Community Supported tvOS for Core, Auth, Database and Storage. Add tvOS unit tests Add tvOS sample app Update README.md Add tvOS to travis testing * Adds API Test for Email Update (#613) * update Gemfile for Travis (#620) * update Travis to Xcode 9.2 (#619) * Removing an obsolete setting from plist files (#617) * Removing an obsolete setting from plist files * Fixing Unit Tests * Fixing nullability * Fully qualify protoc-generated outputs (#626) * Fully-qualify imports in the protocol compiler output * pbxproj updates from running pod update * New checked-in proto outputs * Port StringPrintf from //base (#624) * Port StringPrintf from //base. Prefer this to approaches based on variadic templates. While the variadic template mechanisms are strictly safer, they result in binary bloat we can't afford. This is essentially the same StringPrintf previously open sourced as a part of protobuf, though updated for C++11 which saves a copy and a temporary buffer on the heap. * Add abseil as a subdirectory of Firestore This saves having to redefine all the libraries that abseil defines as imported libraries. * Rename firebase_firesture_util_log_* targets Cut the log out of the name to reflect that these will get more components besides just logging. * Update tests for firebase_firestore_util renames

Add Community Supported tvOS for Core, Auth, Database and Storage. Add tvOS unit tests Add tvOS sample app Update README.md Add tvOS to travis testing

ryanwilson and others added 20 commits November 13, 2017 13:32

Initial tvOS project sample. (#447)

f59bb1b

Initial tvOS project sample.

Fix Storage compilation (#448)

7d25a7c

Firebase Core and its unit tests for tvOS (#449)

cd0fc65

* Core Unit Tests for tvOS * tvOS 10 and shared schemes * copyrights * style fixes

Storage tvOS Sample (#450)

cde66b1

* Example using Firebase Storage. * Remove unnecessary AppDelegate methods * Fixed comment on constants

Add tvOS Storage unit testing (#451)

bbf4fa7

Build and unit test Database for tvOS (#452)

b2894ff

* Build and unit test Database for tvOS * delete blank line

Added Database sample app (#454)

d0ba341

Remove leveldb workaround and add AllUnitTests_tvOS scheme (#457)

5d4a36b

Auth tvOS build environment + email/password example. (#459)

df28b4e

* Build environment for tvOS Auth * Added Auth example for email and password sign in. * Fixed TARGET_OS_TV check * Cleaned up storyboard

Hopefully fix the Xcode project. (#460)

aed83c6

Various build and test fixes (#461)

719a897

Merge integrated open source build updates from master (#491)

4365bed

Raw Merge

c22f9ae

Fix bad merging of deleted files

ca39414

Fix up xcode build after merge

7d37d11

Xcode 9.2 sample updates

6098d2b

Merge pull request #586 from firebase/pb-tv-merge-master2

bfcfa0a

Merge master to tvOS master

Update README for tvOS and include in travis (#587)

85c7a3b

Fix copywrite header (#589)

45558a5

Merge remote-tracking branch 'origin/tvOS_master' into pb-merge-tvOS

548832d

googlebot added the cla: yes label Dec 21, 2017

paulb777 added 2 commits December 21, 2017 17:09

Workaround pod lib lint warning

6b630d5

kick travis

b9084fb

ryanwilson approved these changes Dec 22, 2017

View reviewed changes

trailing spaces

dd5ba1f

paulb777 requested review from schmidt-sebastian and protocol86 December 22, 2017 18:41

paulb777 assigned protocol86 and schmidt-sebastian Dec 22, 2017

schmidt-sebastian suggested changes Dec 22, 2017

View reviewed changes

Address comments from schmidt-sebastian.

aaa5aa0

schmidt-sebastian approved these changes Jan 2, 2018

View reviewed changes

Fixed false caching comment, updated path documentation

ed44a47

protocol86 reviewed Jan 2, 2018

View reviewed changes

ryanwilson added 2 commits January 3, 2018 13:32

Address review comments.

5d416a9

Merge branch 'master' into pb-merge-tvOS

307a557

protocol86 approved these changes Jan 3, 2018

View reviewed changes

paulb777 merged commit f08b504 into master Jan 3, 2018

paulb777 deleted the pb-merge-tvOS branch January 3, 2018 20:12

paulb777 mentioned this pull request Jan 3, 2018

FR: Adding tvOS Support #10

Closed

firebase locked and limited conversation to collaborators Nov 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Community Supported tvOS #590

Add Community Supported tvOS #590

paulb777 commented Dec 21, 2017

ryanwilson left a comment

schmidt-sebastian left a comment •

edited

Loading

schmidt-sebastian Dec 22, 2017

ryanwilson Dec 22, 2017

schmidt-sebastian Dec 22, 2017

ryanwilson Dec 22, 2017

schmidt-sebastian Dec 22, 2017

ryanwilson Dec 22, 2017

schmidt-sebastian Dec 22, 2017

ryanwilson Dec 22, 2017

schmidt-sebastian Jan 2, 2018

schmidt-sebastian Dec 22, 2017

ryanwilson Dec 22, 2017

schmidt-sebastian Jan 2, 2018 •

edited

Loading

ryanwilson Jan 2, 2018

schmidt-sebastian Dec 22, 2017

schmidt-sebastian Jan 2, 2018 •

edited

Loading

protocol86 Jan 2, 2018

ryanwilson Jan 3, 2018

protocol86 Jan 3, 2018

protocol86 Jan 2, 2018

ryanwilson Jan 3, 2018

protocol86 Jan 2, 2018

ryanwilson Jan 3, 2018

protocol86 Jan 2, 2018

ryanwilson Jan 3, 2018

protocol86 Jan 3, 2018

protocol86 Jan 2, 2018

ryanwilson Jan 3, 2018

mariohahn commented Jan 4, 2018

Add Community Supported tvOS #590

Add Community Supported tvOS #590

Conversation

paulb777 commented Dec 21, 2017

ryanwilson left a comment

Choose a reason for hiding this comment

schmidt-sebastian left a comment • edited Loading

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

schmidt-sebastian Jan 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmidt-sebastian Jan 2, 2018 • edited Loading

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

mariohahn commented Jan 4, 2018

schmidt-sebastian left a comment •

edited

Loading

schmidt-sebastian Jan 2, 2018 •

edited

Loading

schmidt-sebastian Jan 2, 2018 •

edited

Loading