Javascript - On mouseup check if any of the parents has/includes specific id

Question

On my page I have a mouseup eventlistener on document. I want to be able catch if the clicked elements has a specific id in any of their parents.

What I'm doing now is to loop over the e.path and only push the ids to an array and from there check if the array includes that id (the id is: getme). You can see the code below.

I feel this is not optimal or the right way of doing this. How can I make it so I don't need to each time loop over all the e.path and push to the array and than do the include.

document.addEventListener("mouseup", function(e){
 console.log('Document was clicked - Document click fucntion comes here');
 console.log(e.path);
 idsArr = [];
 const allPaths = e.path;
 allPaths.forEach((path)=>{
 if (path.id){
 idsArr.push(path.id);
 }
 });
 console.log(idsArr);
 if (idsArr.includes('getme')){
 console.log('Array Includes the ID - Specific ID click fucntion comes here');
 }
});

.getme{background:blue;width:300px;height:400px;}.dontneed{width:200px;height:300;float:left;}

 <div class="getme" id="getme">
 <div class="parent1">
 <div class="parent2">
 <p>Im pragraf</p>
 <div class="parent3">
 <button>Im button</button> 
 </div>
 </div>
 <img src="https://picsum.photos/200/300">
 </div>
</div>
<div class="dontneed">
 <img src="https://picsum.photos/200/300">
</div>
<p>Dont need</p>

Any suggestion or feedback is appreciated!

Answer 1

Review

The code is very inflexible, too complex, and has very poor naming.

Inflexible

The code will force you write a new function for each id you want action on. It is not at all reusable without modification, which means it will need to be tested each time you use it.

Overly complex

Creating a copy of part of the path then to search that copy for a match is inefficient as the match can be found in one expression.

const getMeClicked = e.path.some(el => el === "get me"); // bool

Poor naming.

You are confusing paths, elements, and ids. You are also naming an array that will never contain more than one item.

• idsArr is not an array of ids, it is an array of elements with an id. There can only ever be one copy of an id (if page is not in quirks mode). Thus this should not be an array.

• allPaths There is only one path. There is no need to hold a second reference to event.path

• path (in forEach callback) The for each is iterating elements not paths, the name should be element, or common abbreviation el

Use Strict_mode

As pointed out in the other answer you have used an undeclared variable. This is very dangerous.

To avoid this mistake always write your JS in Strict_mode

Note that if using modules you don't need to use the directive as they can only run in strict mode.

Rewrite

Rewrite to provide identical functionality (ignoring and modifying some logs)

It uses Array.some so that it can stop iterating when the id is found

"use strict"; // Must be at top of JS tag or file.
document.addEventListener("click", e => {
 console.log("Document was clicked");
 e.path.some(el => {
 if (el.id === "getMe") {
 console.log("Includes the ID");
 return true;
 }
 });
});

However this is rather clunky and un-reusable so below is an re-design

Example redesign

Uses a function factory to return an event callback that will branch calls depending on the ids found in the path.

/* Function factory onMatchId 
 id String Id to Match in event.path
 onId Callback to call if path contains id [optional]
 onNotId Callback to call if path does not contain id [optional]
 onAny Callback to call on any event [optional]
 Callback signature 
 callback(e, id) 
 e is event object. 
 id is id string to match;
*/
const onMatchId = (id, onId, onNotId, onAny) => e => 
 (onAny?.(e, id), (e.path.some(el => el.id === id) ? onId : onNotId)?.(e, id));

Simple example

A callback to match one id in path and a callback for all clicks

;(()=> {
"use strict";
const onMatchId = (id, onId, notId, any) => e => 
 (any?.(e, id), (e.path.some(el => el.id === id) ? onId : notId)?.(e, id));
 
 
/* callbacks */
var clickNum = 0;
const anyClk = () => console.log("Document was clicked");
const clk = () => console.log("ID: getme clicked");
/* simple usage example */
document.addEventListener("mouseup", onMatchId("getme", clk, undefined, anyClk));
})();
 

Text not of getme <button>Button not of getme</button>
<div id="getme">
 <div id="a2">
 <div id="a1">
 I'm text child of a1 a2 getme
 <button>Child of a1 a2 getme</button>
 </div>
 <button>Child of a2 getme</button>
 </div>
 <button>Child of getme button</button>
</div>

Complex example

Includes many callbacks, one for all clicks, then for "getme" clicked and not clicked, id "a2" clicked and not clicked, and for id "a1" only when clicked

;(()=> {
"use strict";
const onMatchId = (id, onId, notId, any) => e => 
 (any?.(e, id), (e.path.some(el => el.id === id) ? onId : notId)?.(e, id));
 
 
/* callbacks */
var clickNum = 0;
const anyClk = (e, id) => (console.clear(), console.log("Click num: " + clickNum++));
const clk = (e, id) => console.log("ID: " + id + " clicked");
const notClk = (e, id) => console.log("Not ID: " + id + " clicked");
document.addEventListener("click", onMatchId("getme", clk, notClk, anyClk));
document.addEventListener("click", onMatchId("a2", clk, notClk));
document.addEventListener("click", onMatchId("a1", clk));
})();
 

<div id="getme">
 <div id="a2">
 <div id="a1">
 I'm text child of a1 a2 getme
 <button>Child of a1 a2 getme</button>
 </div>
 <button>Child of a2 getme</button>
 </div>
 <button>Child of getme button</button>
</div>
Text not of getme <button>Button not of getme</button>

Answer 2

I feel this is not optimal or the right way of doing this. How can I make it so I don't need to each time loop over all the e.path and push to the array and than do the include.

Another approach would be to use the element method matches to check if the target of the event is the element with the specific id or a child element.

document.addEventListener("mouseup", function(e){
 console.log('Document was clicked - Document click fucntion comes here');
 if (e.target.matches('#getme, #getme *')) {
 console.log('Array Includes the ID - Specific ID click fucntion comes here');
 }
});

.getme{background:blue;width:300px;height:400px;}.dontneed{width:200px;height:300;float:left;}

<div class="getme" id="getme">
 <div class="parent1">
 <div class="parent2">
 <p>Im pragraf</p>
 <div class="parent3">
 <button>Im button</button> 
 </div>
 </div>
 <img src="https://picsum.photos/200/300">
 </div>
</div>
<div class="dontneed">
 <img src="https://picsum.photos/200/300">
</div>
<p>Dont need</p>

---

The array idsArr is not declared with any scope keyword (e.g. var, let, const) and thus it is considered a global variable. It is recommended to avoid global variables.

As the snippet below demonstrates the idsArr could be derived using the array methods map and filter

document.addEventListener("mouseup", function(e){
 console.log('Document was clicked - Document click fucntion comes here');
 
 const allPaths = e.path;
 const idsArr = e.path.map((path) => path.id).filter(id => id)
 
 console.log('idsArr: ', idsArr);
 if (idsArr.includes('getme')){
 console.log('Array Includes the ID - Specific ID click fucntion comes here');
 }
});

.getme{background:blue;width:300px;height:400px;}.dontneed{width:200px;height:300;float:left;}

<div class="getme" id="getme">
 <div class="parent1">
 <div class="parent2">
 <p>Im pragraf</p>
 <div class="parent3">
 <button>Im button</button> 
 </div>
 </div>
 <img src="https://picsum.photos/200/300">
 </div>
</div>
<div class="dontneed">
 <img src="https://picsum.photos/200/300">
</div>
<p>Dont need</p>

• Rubioli: Appreciate it Sam :)
• Rubioli: which of the solutions are the "best" to use?
• Sᴀᴍ Onᴇᴌᴀ: The answer may depend on requirements... for example, I would say using the .matches() method might be best unless a team-mate is opposed to that approach or there are many target ids to look for. I also considered asking if you are familiar with event bubbling, and checking if the event handler could be added to the element with id getme instead of the document...
• Rubioli: Thanks a lot mate. Yes at first I tested with to adding the eventlistener to getme but because of some requirements I need it to be on on document.