Skip to content

Use event listeners to select a row of times#29

Open
andytlr wants to merge 14 commits into
gh-pagesfrom
use-eventlisteners
Open

Use event listeners to select a row of times#29
andytlr wants to merge 14 commits into
gh-pagesfrom
use-eventlisteners

Conversation

@andytlr

@andytlr andytlr commented Sep 6, 2014

Copy link
Copy Markdown
Owner

Fixes #27

  • Get whole row selecting on click
  • Toggling of row selection
  • Don't let the current time be selectable.
  • Set var interval for the things that still use intervals to something reasonable. like 1000.
  • Turn application cache back on.

@andytlr

andytlr commented Sep 6, 2014

Copy link
Copy Markdown
Owner Author
hourNode.addEventListener("click", addSelectedClass, true);
hourNode.addEventListener("click", hideOrShowEmailButton, true);
hourNode.addEventListener("click", selectOtherCellsInRow(index, selectedIndex, hourNode), true);

selectOtherCellsInRow doesn't run on click. It's still happening on the interval timer. Where as addSelectedClass and hideOrShowEmailButton are both running instantly.

This is really obvious if I set the interval timer to two seconds:

delay

The email button is added instantly, and the clicked cell is highlighted instantly. But the row is only highlighted after a delay.

@samcambridge

Copy link
Copy Markdown

Are index and selectedIndex defined at this point?

@andytlr

andytlr commented Sep 7, 2014

Copy link
Copy Markdown
Owner Author

They should be. I had to pass them as arguments to make it work inside the function.

@samcambridge

Copy link
Copy Markdown

They may not be at the time the event listener is set up.

Add "debugger" (without the quotes, can't do markdown on my phone) above the line the event listener is on and reload the page with your chrome console open.

It should pause the script for you and you'll be able to type variable names in the console, if they return as undefined they're not set.

Alternately if they aren't set you probably would be getting JavaScript errors in your browser console.

@andytlr

andytlr commented Sep 7, 2014

Copy link
Copy Markdown
Owner Author

Thanks @samcambridge

selectedIndex = undefined; until this runs:

function addSelectedClass() {
  this.classList.add("selectedhourforsharing");
  selectedIndex = index;
}

Which is run by this on click:

hourNode.addEventListener("click", addSelectedClass, true);

Which is above:

hourNode.addEventListener("click", selectOtherCellsInRow(index, selectedIndex, hourNode), true);

So I thought by the time selectOtherCellsInRow ran, it would be aware of what selectedIndex was.

@samcambridge

Copy link
Copy Markdown

Everyone loves Race conditions!

@andytlr

andytlr commented Sep 7, 2014

Copy link
Copy Markdown
Owner Author

Ugh, right. Thanks.

@samcambridge

Copy link
Copy Markdown

Do you need an event listener for selectOtherCellsInRow or can you just call it directly from addSelectedClass?

@levibuzolic

Copy link
Copy Markdown
hourNode.addEventListener("click", selectOtherCellsInRow(index, selectedIndex, hourNode), true);

That doesn't run selectOtherCellsInRow on click, that will run the result of selectOtherCellsInRow on click. Which my guess isn't what you want.

When you write your event listener like that, selectOtherCellsInRow(index, selectedIndex, hourNode) is run once, immediately when the file is first parsed, and then never again. The return value of whatever that method is, will be what is run each time the event listener is called.

What I assume you want is this:

hourNode.addEventListener("click", function(){ selectOtherCellsInRow(index, selectedIndex, hourNode) }, true);

This will run the anonymous function every time the event listener is called, which will in turn run the selectOtherCellsInRow(index, selectedIndex, hourNode) method.

@levibuzolic

Copy link
Copy Markdown

To try and explain this a little better, take this example:

someFunction = function(){
  alert('Called');
};

node.addEventListener( 'click', someFunction );
// This will alert every time the node is clicked

node.addEventListener( 'click', someFunction() );
// This will alert once on file load, and never again even if we click the node

@levibuzolic

Copy link
Copy Markdown

One thing to note is that this may not fix your race condition, but race-condition-or-not with the way you're calling the function currently it's never going to work as expected.

@andytlr

andytlr commented Sep 7, 2014

Copy link
Copy Markdown
Owner Author

Thank you @levibuzolic that makes a lot more sense. Unfortunately I still haven't got it to work. I tried:

function selectOtherCellsInRow(index, selectedIndex, hourNode) {
  if (index == selectedIndex) {
    debugger
    hourNode.classList.add("selectedhourforsharing");
  } else {
    hourNode.classList.remove("selectedhourforsharing");
  }
}

hourNode.addEventListener("click", function(){ selectOtherCellsInRow(index, selectedIndex, hourNode) }, true);

If I click on the third row, in the debugger I get:

index
2
selectedIndex
2

Which is correct. The index is 2 and the selectedIndex is also 2. So I don't think it's a race condition.

It's like the function isn't aware of all hourNodes. Just the one I clicked on. If I take the if statement out of the function the debugger works quite differently. Inside the function you can see that it debugs the function and that's it. But if the if statement is outside the function it keeps looping through each hour because it's inside the forEach above:

[].forEach.call(hours, function(hourNode, index) {
...
}

@levibuzolic

Copy link
Copy Markdown

I'll try running the code locally tomorrow and take a proper look. There were a few things I saw earlier that could be tidied up, but I'll have to run it locally to get a better feel for what it's all doing. 👍

@andytlr

andytlr commented Sep 7, 2014

Copy link
Copy Markdown
Owner Author

❤️ thank you.

Conflicts:
	assets/app.js
	homeslice.appcache
@andytlr andytlr changed the title Use Event Listeners Use event listeners to select a row of times Sep 8, 2014
@samcambridge

Copy link
Copy Markdown

you still working on this mate? (finally came back to github :P)

@levibuzolic

Copy link
Copy Markdown

👋 HI SAM

@andytlr

andytlr commented Nov 5, 2015

Copy link
Copy Markdown
Owner Author

Oh hey guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Look into using event listeners instead of intervals

3 participants