Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

JavaScript function inside eventListener only firing on second click

Hi, I'm building an online store using vanilla JS, Firebase and Stripe. When the store page loads, my app pulls any current stock from Firestore, parses it to JS objects, then renders the stock in html. Once the html has rendered, event listeners are attached to the "buy now" buttons and when these are clicked, the app will take the item id of the item clicked, check it's in Firestore, parse it to a JS object, create a new Order object from the item object, then call drawOrderModal() using that order object.

Currently, the correct item gets logged to the console on every button click. The event listener IS currently loading the order modal, but ONLY on the second click. I don't get any error messages in the console.

If i click item001 the first time (order modal doesn't load), then click item002 the second time, item001 is the one that displays in the order modal.

Can anyone see my mistake?

Here is my code:

/** 
 * Creates item objects from Firestore docs; 
 * adds item objects to stockItemsArray; 
 * draws item objects on store page;
 * attaches event listeners to all `${.buyButton}`.
 */
Stock.shared.addItemsToStock()
    .then( () => {
        Stock.shared.drawSTOREitems();
        Stock.shared.drawItemModals();
        // event listeners for buyNow buttons
        const buyNowBtns = document.querySelectorAll(".buyButton");
        buyNowBtns.forEach( btn => {
            btn.addEventListener("click",function(event){
                let itemID = event.target.parentElement.parentElement.parentElement.getAttribute("data-id");
                console.log(itemID + "button clicked");
                firestoreStock.doc(itemID).get()
                .then( doc => {
                    if (doc.exists) {
                        let item = Item.parseDocToItem(doc);
                        if (item) {
                            let order = new Order(item);
                            console.log(order);
                            drawOrderModal(order);
                        } else {
                            console.log("Cannot parse - item empty");
                        };
                    } else {
                        console.log("No such document.");
                    };
                });
            });
        });
    })
    .catch( err => {
        // display error message to users
        let errorMessageDiv = document.createElement("div");
        let errorMessage = document.createElement("p");
        errorMessage.textContent = "There was a problem loading store stock. We are working to fix this. Please come back again later.";
        clothesRack.appendChild(errorMessageDiv);
        errorMessageDiv.appendChild(errorMessage);
        console.log(err);
    });

1 Answer

Eric M
Eric M
11,545 Points

Hi Lana,

I'm no JavaScript expert and I've never used Firestore before, but I would do two things definitely and one thing optionally if I were debugging this.

1. Follow the pattern for .then() from the official docs https://github.com/firebase/snippets-web/blob/8a72018c094b2d48ecbf1690787b94a01ee8fab4/firestore/test.firestore.js#L513-L524 e.g.

firestoreStock.doc(itemID).get()
                    .then(function(doc) => 
                    {
                        if (doc.exists) 
                        {
                                                ...

rather than

firestoreStock.doc(itemID).get()
                    .then(doc => 
                    {
                        if (doc.exists) 
                        {

2. Add more console.log statements for debugging and/or step through the running code with a debugger. For instance I'd like to see the result of a console.log(doc) immediately prior to if (doc.exists) and the same for just before if (item).

You're logging the itemID correctly, so why are these conditional checks failing? Need to figure that out and more logging is likely to help. Are the logs from your else checks firing? If not, do these logs get executed or is the then() statement not actually executing? Check the firestore docs and community to see if anyone else has encountered this.

Optional Refactor your code to be simpler and easier to read. If you don't have to use anonymous functions and promise chains, don't - they can make things difficult to debug.

For instance I'd start by moving the function on the addEventListener call into its own function rather than lumping it in there. Simpler code is easier to debug. Smaller units of code are easier to test and debug. Simplify and separate where ever possible.

Finally Good luck! Coding real world applications always comes with challenges.

Cheers,

Eric

Thanks very much for this, Eric - I definitely wanted to make this simpler and easier to read, I just wasn't sure how! E.g., I need the event listener to get the itemID, and then I need to use that ID to make a bunch of other things happen. How can I make the event listener return the itemID and then reuse it in a separate function?

Eric M
Eric M
11,545 Points

No worries Lana. Just pass the values you need as arguments.

For instance, here's how I'd start the refactor:

function modalHandler(doc)
{
    if (doc.exists) 
    {
        let item = Item.parseDocToItem(doc);
        if (item) 
        {
            let order = new Order(item);
            console.log(order);
            drawOrderModal(order);
        } 
        else
        {
            console.log("Cannot parse - item empty");
        };
    } 
    else
    {
        console.log("No such document.");
    };
}

getItemFailureHandler(reason)
{
    console.log("Unable to retrieve itemID from firestoreStock, reasaon: " + reason)
}

function clickHandler(event)
{
    let itemID = event.target.parentElement.parentElement.parentElement.getAttribute("data-id");
    console.log(itemID + "button clicked");

    firestoreStock.doc(itemID).get().then(
        function(doc) {modalHandler(doc)},
        function(reason) {getItemFailureHandler(reason)}
    );
}


/** 
 * Creates item objects from Firestore docs; 
 * adds item objects to stockItemsArray; 
 * draws item objects on store page;
 * attaches event listeners to all `${.buyButton}`.
 */
Stock.shared.addItemsToStock()
    .then( () => 
    {
        Stock.shared.drawSTOREitems();
        Stock.shared.drawItemModals();
        // event listeners for buyNow buttons
        const buyNowBtns = document.querySelectorAll(".buyButton");
        buyNowBtns.forEach(btn => {
            btn.addEventListener("click", function(event) {clickHandler(event)} );
        });
    })
    .catch( err => {
        // display error message to users
        let errorMessageDiv = document.createElement("div");
        let errorMessage = document.createElement("p");
        errorMessage.textContent = "There was a problem loading store stock. We are working to fix this. Please come back again later.";
        clothesRack.appendChild(errorMessageDiv);
        errorMessageDiv.appendChild(errorMessage);
        console.log(err);
    });

Keep in mind I haven’t tested any of this so there may be more tweaks you need to make beyond this separation.

The MDN Docs are a great reference for different examples of how to structure various calls. Including event listeners and promises.

It might also be worth revisiting some of the Treehouse videos that cover functional abstraction.

Best of luck,

Eric

Thanks so much, Eric - your code works and is a lot easier to understand. When I click on "buy now", modalHandler(doc) is being called and the new order object is being logged to the console. drawOrderModal(order) is also being called (I've tested by adding another console log inside this function), so it seems the order modal is being drawn but not displayed. I'll have to look into that further.

You've really helped a lot, and you're right, I was forgetting about event handlers & how to structure them - I'll brush up on this. Thanks again!

Eric M
Eric M
11,545 Points

Glad it helped Lana!

I've also just noticed I didn't add function to the function definition for getItemFailureHandler in my example refactor

Hope you can find the bug. Sounds tricky!

Best of luck

Yes that's all good, I added the function keyword in there when I copied your code :) A little tricky, but I probably just haven't set something up properly with the modal - I'll let you know what the issue is when I figure it out!