Moving code from PHP to .js file, code not functioning

Question

I picked up a project that another developer built. He has some of the JavaScript on the actual php page which is making debugging difficult.

<button type="submit" onclick="doFunction(event)">Done</button>

I have tried to move it to a javascript file:

$( document ).ready(function() {
    function doFunction(e){}
});

and removing it from the PHP page but now it's not firing. Why is it not working now and how do I fix it?

did you load the external file? ``? the browser isn't going to magically realize that you moved the code elsewhere. — Marc B, Dec 07 '15 at 16:28
`e` is undefined within your `.ready()`. I suggest you look at your js console for errors. — Marc B, Dec 07 '15 at 16:29
Did you leave the `` or is that the previous solution that you've removed? — rockerest, Dec 07 '15 at 16:30

score 4 · Accepted Answer · edited May 23 '17 at 12:23

4

You've defined function doFunction(e){} inside another function.

That locally scopes it. It is not a global. It is not accessible from onclick attributes (since they aren't defined inside the same function).

Don't use onclick attributes, they have many problems, bind your event handlers with JavaScript instead.

Since you are using jQuery:

$('button').on('click', doFunction);

You can use a more specific selector if you like.

That said, almost anything you want to do with a submit button is clicked is better done when a form is submitted.

$('form').on('submit', doFunction);

edited May 23 '17 at 12:23

Community

answered Dec 07 '15 at 16:30

Quentin

How can I make it globally accessible? – David Tunnell Dec 07 '15 at 16:30
1

Don't. Globals are evil. – Quentin Dec 07 '15 at 16:31
@DavidTunnell define `function doFunction(e)` in the global scope and not inside the `$( document ).ready` function call. If you don't want to pollute the global scope, you can define an object in the global scope with `doFunction` as one of it's properties. – kayasky Dec 07 '15 at 16:32
1

As a trick, you can do : `window['doFunction'] = function(e){}` But this is bad practice – Robin Choffardet Dec 07 '15 at 16:32
1

@DavidTunnell, While I agree with the sentiment of Quentin on not making it global, some projects in the real world don't have the budgets to do things the right way. this project may have hundreds of these calls baked into the HTML. And yes, I'd use Robin's method or `window.doFunction = function(e){ ... };` – Joseph Marikle Dec 07 '15 at 16:34

1 Answers1