0

I'm trying to implement a JavaScript function for a HTML page that simply reveals text on mouse down. I want to apply this multiple times so don't want to have to create a function for each event.

The below code works fine...

function showSkillInfo() {
    if (skill1Info.style.display === 'none') {
        skill1Info.style.display = 'grid';
    } else {
        skill1Info.style.display = 'none';
    };
};
skill1.onmousedown = showSkillInfo;

But this block of code seems to have no response...

function showSkillInfo2(skill) {
    if (skill.style.display === 'none') {
        skill.style.display = 'grid'
    } else {
        skill.style.display = 'none'
    };
};
skill1.onmousedown = showSkillInfo2(skill1Info);

Can anyone tell me why the second function isn't working the same way the first function is?

Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122
  • can you add your relevant HTML and reproduce the problem? you are using 2 different examples here, the first one uses `skill1Info` which I assume was already defined outside your function, and the second example sends the element as an argument `skill` – Chris G May 08 '23 at 20:04
  • Please [edit] your question to include a [mre] showing the second block not working. You can use a [Stack Snippet](https://meta.stackoverflow.com/q/358992/215552) to include both the JavaScript and HTML in a runnable snippet. – Heretic Monkey May 08 '23 at 20:04
  • `onmousedown` takes a function reference. In the first block, you are specifying `showSkillInfo`, which is a function reference. In the second block, you are specifying `showSkillInfo2(skill1Info)`, which is undefined, since `showSkillInfo2` does not return anything. You would need to change it to `showSkillInfo2.bind(skill1Info, skill1Info)` or `function() { showSkillInfo2(skill1Info); }` for it to work correctly. – Heretic Monkey May 08 '23 at 20:08
  • Does this answer your question? [Assigning .onmousedown with () runs the function when assigned , work arounds](https://stackoverflow.com/questions/17246502/assigning-onmousedown-with-runs-the-function-when-assigned-work-arounds) – Heretic Monkey May 08 '23 at 20:10
  • can't you just add an `eventListener` to all elements you want to affect with the `onmousedown` instead? – Chris G May 08 '23 at 20:13
  • 1
    [Don't use on(...) event handling](https://developer.mozilla.org/en-US/docs/Web/Events/Event_handlers), it was the web's first attempt at JS events for HTML pages, almost 30 years ago, and was superseded by [addEventListener](https://developer.mozilla.org/en-US/docs/Web/Events/Event_handlers#eventtarget.addeventlistener) less than 5 years later and has be the "right" way to handle events ever since. Use that instead. As for the question: because you're calling your function in code block 2 and binding the _result_ of that call, instead of binding the _function_ that needs to run. – Mike 'Pomax' Kamermans May 08 '23 at 20:16
  • Since it sounds like you have multiple skills that you want to click on you may want to consider [event delegation](https://dmitripavlutin.com/javascript-event-delegation/) which allows you to attach one listener to a containing element (sometimes `document`) and have that listen for events from its child elements as they "bubble up" the DOM. – Andy May 08 '23 at 20:39

2 Answers2

1

The .onmousedown attribute is a callback function called when a specific event is triggered (mouse down in that case, but all other events work the same way). This means that a reference to a function or a lambda function must be specified for it to work. The first example is indeed a reference to a function and works as expected. The second one, on the other hand, is a call to a function and it is its return value (undefined) that will be affected to skill1.onmousedown. For it to work as intended, you need to specify a new function that will be called with the right parameter. The best way to do this is using lambda functions, like this: skill1.onmousedown = () => showSkillInfo2(skill1Info); This way, skill1.onmousedown has the value of a function with no parameter, and it can be called when the associated event is triggered.

0

This line: skill1.onmousedown = showSkillInfo2(skill1Info); is being called immediately, before skill can be defined, so it doesn't work.

What you'll need to do is put that line within another function and that other function will be assigned as the mousedown callback function.

Also, you don't need two classes here. You can just have a default class that is in effect all the time, and then change it to the new class upon mousedown.

Lastly, you shouldn't use HTML event attributes to set up events as this is a nearly 30 year old technique from before we had standards. Instead, separate your HTML and JavaScript.

See the comments inline below:

function showSkillInfo(event) {
  // First, determine if the event occured on an element you care to handle
  if(event.target.classList.contains("skill")){
    // Just use the .toggle() method to toggle the use of a class.
    // No if statement needed.
    // event.target references the actual element that triggered the event
    event.target.classList.toggle("grid");
  }
};


// We'll just set up mousedown and mouseup events on a parent element
// of the possible elements that may need to be handled.
// by wrapping your callback inside of another function, you get the
// ability to set up the callback without invoking the "real" code 
// that should wait for the event to happen. You can also pass the 
// automatically provided reference to the event that triggered the
// callback to the function.
document.querySelector("div").addEventListener("mousedown", function(event) {
  showSkillInfo(event);
});

document.querySelector("div").addEventListener("mouseup", function(event) {
  showSkillInfo(event);
});
.skill {
  color:aqua;
  cursor:pointer;
}

.grid { 
  /* This is sample CSS. Replace with yours.  */
  color:red;
}
<div>
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer <span class="skill">took a galley</span> of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised <span class="skill">in the 1960s</span> with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker <span class="skill">including versions</span> of Lorem Ipsum.
</div>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71