1

What I am doing is trying to build a ticket registration app in the new SharePoint Framework. Basically, the app will read items from a list of Games titled "Games", if there are any tickets left for that games allotment it will offer a few input fields and a register button. Users will fill out required fields, and upon clicking register it will submit the number of tickets requested and the user's info to a separate attendees list, and deduct the requested amount from the allotted tickets in the games list.

I have added an event handler to a button on each list item that is rendered in my list. When clicking the button it always updates the first list item, instead of the list item the button is actually tied to. I was wondering if someone could look at my code and tell me where I am going wrong?

https://github.com/roberts2727/KSETicketSignups/blob/master/src/webparts/kseTicketSignups/KseTicketSignupsWebPart.ts

0

I tried to read your code

You are making it hard for yourself by mixing React with ES3 coding patterns.

Why addEventListener? if React has: https://facebook.github.io/react/docs/handling-events.html

Ditch ES3 code

This is a mix of ES3 and modern programming:

private setButtonsEventHandlers(): void {
    const webPart: KseTicketSignupsWebPart = this;
    const buttons: NodeListOf<Element> = this.domElement.querySelectorAll(`button.${styles.button}`);

    for (let i: number = 0; i < buttons.length; i++) {
        const button: Element = buttons.item(i);
        button.addEventListener('click', () => { webPart.updateItem(); });
    }
}

This is ES6 (if you want to add the Listeners yourself) of the top of my head, not tested for syntax

private setButtonsEventHandlers(): void {
    this.domElement.querySelectorAll("button."+styles.button)
        .forEach(button=>button.addEventListener('click', () => this.updateItem());
}
  • no need for webPart, arrow functions pass scope

  • No need for a String literal, "button."+styles.button is shorter, better to read and executes faster

  • You are using forEach in other parts of your code, then loose that 1980s for loop here as well

Less code === less errors === easier to spot where you go wrong


JSX is great for creating code like this:

let html: string = '';
items.forEach((item: ISPList) => {


    if (item.Remaining > 0) {
        html += `
        <ul class="${styles.list}">
            <li class="${styles.listItem}">
                <span class="ms-font-l">${item.Title}<br>${item.Day}<br>${item.jcaa}<br>Tickets Allotted: ${item.Alloted}<br>Tickets Remaining: ${item.Remaining}<br>
                  <button class="${styles.button} update-Button">
            <span class="${styles.label}">Register!</span>
          </button>
                </span>
            </li>
        </ul>`;
    }
    else {
        html += `
        <ul class="${styles.list}">
            <li class="${styles.listItem}">
                <span class="ms-font-l">${item.Title}<br>${item.Day}<br>${item.jcaa}<br>Tickets Allotted: ${item.Alloted}<br>Tickets Remaining: ${item.Remaining}<br>Sorry, Game is Closed.</span>
            </li>
        </ul>`;
    }
});
const listContainer: Element = this.domElement.querySelector('#spListContainer');
listContainer.innerHTML = html;

You are dupplicating HTML and because it is hard to read you are creating ULs with just one LI (or is that on purpose?)

Beside .forEach() and .map() there are more fantastic ES6 Array methods.

Take .reduce, which reduces an Array into any other datatype.

So here goes your ES6 version: of the top of my head

this.domElement.querySelector('#spListContainer').innerHTML = 
  items.reduce((html: string, item: ISPList) => {
      let remaining = `<button class="${styles.button} update-Button">
                         <span class="${styles.label}">Register!</span>
                       </button>`;
      if (item.remaining === 0) remaining = 'Sorry, Game is Closed.';
      return html += `<li class="${styles.listItem}">
                <span class="ms-font-l">${item.Title}
                  <br>${item.Day}
                  <br>${item.jcaa}
                  <br>Tickets Allotted: ${item.Alloted}
                  <br>Tickets Remaining: ${item.Remaining}
                  <br>${remaining}
                </span>
            </li>`;
    }, `<ul class="${styles.list}"><!--Items go here-->`) + "</ul>";

Now I understand your code, I think...

You want to know which button was clicked... because it is an eventListner updateItem gets an event as parameter, so you can check event.target

private setButtonsEventHandlers(): void {
    this.domElement.querySelectorAll("button."+styles.button)
        .forEach(button=>button.addEventListener(
                 'click', (event) => this.updateItem(event.target));
}

You then get the clicked Button in updateItem

And you probably want to JSX the item id on that button:

      let remaining = `<button itemid="${item.id}" class="${styles.button} update-Button">

I think

  • after implementing the changes you suggested I have a few typescript errors. [10:25:42] Error - typescript - src\webparts\kseTicketSignups\KseTicketSignupsWebPart.ts(106,67): error TS1005: ')' expected. [10:25:44] Error - typescript - src\webparts\kseTicketSignups\KseTicketSignupsWebPart.ts(105,9): error TS2339: Property 'forEach' does not exist on type 'NodeListOf<Element>'. [10:25:44] Error - typescript - src\webparts\kseTicketSignups\KseTicketSignupsWebPart.ts(106,37): error TS2346: Supplied parameters do not match any signature of call target. – Robert Woods May 27 '17 at 16:46
  • It does not like forEach on the type 'NodeListOf<Element>', this is why I reverted back to mixing code earlier. I did push to GitHub if you want to see the full code after changes. github.com/roberts2727/KSETicketSignups/blob/master/src/… – Robert Woods May 27 '17 at 16:49
  • Use the ES6 spread operator to convert to an array [...nodeList] – Danny '365CSI' Engelman May 28 '17 at 7:37
  • After looking at different examples of converting a nodeList to an array this is what I was finally able to get working: private setButtonsEventHandlers(): void { var arr = Array.from(this.domElement.querySelectorAll("button."+styles.button)); arr.forEach(button=>button.addEventListener( 'click', (event) => this.updateItem(event.target))); } One last issue I am running into is the last error I mentioned, message: 'Supplied parameters do not match any signature of call target.' at: '107,38' source: 'ts' – Robert Woods May 28 '17 at 16:29
  • That is a TS warning,, not and ECMAscript error, probably the number of parameters for the function (use ? in TS to declare optional parameters) – Danny '365CSI' Engelman May 29 '17 at 14:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.