Skip to content

Gh login km #48

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Gh login km #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49a100e
added logout buttons and event listeners
kammitama5 Jun 14, 2017
070d199
changing buttons to firebase event listeners
kammitama5 Jun 14, 2017
3c9d9bb
added onAuthStateChanged function
kammitama5 Jun 14, 2017
6ef1658
add username and profilephoto variables
kammitama5 Jun 14, 2017
d9a756f
document typo :(
kammitama5 Jun 14, 2017
87944f7
first test
kammitama5 Jun 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions firebase-test/firebasetest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,33 @@ var config = {

firebase.initializeApp(config);

var provider = new firebase.auth.GithubAuthProvider();

// Create a JavaScript object for the HTML element that has id="message"
var messageBox = document.getElementById("message");
// Create a JavaScript object for the HTML element that has id="username"
var usernameBox = document.getElementById("username");

// Create Objects for Buttons UserInfo
var userInfoBox = document.getElementById("userinfo");
// Create Objects for Button Login
var loginButton = document.getElementById("login");
// Create Objects for Button Logout
var logoutButton = document.getElementById("logout");

// Get a reference to the root of our database
var dbRef = firebase.database().ref();
// Get a reference to the "greeting" section of our database
var dbGreeting = dbRef.child("greeting");
// Get a reference to the "myname" section of our database
var dbUsername = dbRef.child("myname");

// button function and event listener
document.addEventListener("click", myButton);
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused by this event listener -- did you get to test it? Seems like it wouldn't work right, or if this is leftover code from a temporary experiment, remember to delete it when you make your next edits. See my comments on line 54 and 62 below regarding the event listeners.


function myButton(github){
alert("hi");
}
// Whenever "greeting" value in our database is updated, show the data inside messageBox!
dbGreeting.on("value", function(dataSnapshot) {
messageBox.textContent = dataSnapshot.val();
Expand All @@ -33,3 +48,45 @@ dbUsername.on("value", function(dataSnapshot) {
usernameBox.textContent = dataSnapshot.val();
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I must have missed this in your last pull request -- you actually should have two Firebase event listeners -- one for the greeting, and one for the username. So you should remove line 48, then copy lines 40 to 50, paste a duplicate, and change that copy to listen for changes to your other Firebase reference (dbUsername) and modify your other paragraph (usernameBox) accordingly.

console.log( dataSnapshot.val() );
});

// Event Listener for Login
// Use Firebase with Github Auth to log in the user
firebase.auth().signInWithRedirect(provider).catch(function(error){
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any event listener here though! When should the user be redirected to log in? What event are you listening for from the user, and where exactly is that event going to happen on the HTML page? Take a look at previous examples of JavaScript event listeners and use one as a reference.

// Log any errors to the console
console.log(error);
});


// Event Listener for Logout
// Change your logout button's event listener
firebase.auth().signOut().catch(function(error){
Copy link
Member

Choose a reason for hiding this comment

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

Same issue as on line 54 -- I don't see an event listener here! See my comment on line 54 for this section too. Same questions apply here: when should the user be logged out?

// Log any errors to the console
console.log(error);
});

// onAuthStateChanged
//When user logs in or logs out:
firebase.auth().onAuthStateChanged(function(user){
// If user is now logged in:
if (user){
console.log("User successfully logged in to Firebase!");

//Here: update the paragraph with ID of "userinfo to display Github's Username and Github profile photo "
//Otherwise, if no user currently logged in:
Copy link
Member

Choose a reason for hiding this comment

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

This comment on line 75 should probably be moved down to around the else statement on line 83, or just remove the comment altogether to avoid any confusion

var username = document.getElementById("username");
var profilephoto = document.getElementById("profilephoto");
Copy link
Member

Choose a reason for hiding this comment

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

Line 76 and 77 are trying to access an element in your HTML file with the IDs "username" and "profilephoto", but they don't exist! You want to be accessing the Firebase user object instead here.


// Display Username and Photo
var show = document.getElementById("userinfo").innerHTML;
var show1 = document.getElementById("profilephoto").innerHTML;
Copy link
Member

Choose a reason for hiding this comment

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

What are the show and show variables for here? I'm not sure I follow.... But remember, here you want to change the value of the innerHTML property belonging to that one new paragraph you created earlier for this challenge. By changing its innerHTML property to a new value, you can display not just text inside that paragraph but also more HTML tags, like an image!

}
else
{
console.log("User successfully logged OUT from Firebase!");

// Here: Update the paragraph with ID of "userinfo" to display the message "Not currently logged in."
document.getElementById("userinfo").textContent = "Not currently logged in";
Copy link
Member

Choose a reason for hiding this comment

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

Since you already created a variable for this element on line 21, you may as well use it here instead of generating the JavaScript object again with document.getElementById() a second time -- keep your code DRY (https://en.wikipedia.org/wiki/Don%27t_repeat_yourself)! :)


}

});
9 changes: 8 additions & 1 deletion firebase-test/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@

<body>
<h1>Testing Firebase!</h1>

<button id="login">Log In With Github</button>
<button id="logout">Log Out</button>

<p id="userinfo">Not currently logged in.</p>

<p id="username"></p>
<p id="message"></p>



<script src="https://www.gstatic.com/firebasejs/4.1.2/firebase.js"></script>
<script src="firebasetest.js"></script>
</body>
Expand Down