0

Jquery is not my strong suit. This seems logically correct though it is not working. I have searched everything from find, to closest, to this, etc. I cannot figure out why this is not working.

WHat I am trying to do is add a class to the closest tr when this <span> is clicked.

I get no errors, but I also get no results.

CODE:

$curTable .= "
    <tr bgcolor='#f2e2f2' onmouseover=style.backgroundColor='#FFFFFF'; onmouseout=style.backgroundColor='#f2e2f2';>
        <td>$typeSelect</td>
        <td>$cfilename</td>
        <td><input type='text' size='20' name='cexpDate' class='dp exp' value='$cexpDate' /><script type='text/javascript'>$('.dp').datetimepicker({format:'m/d/Y'});</script></td>
        <td>$catSelect</td>
        <td>$cdateAdded</td>
        <td>$caddedBy</td>
        <td><span class='mod' onclick=\"deleteFile('{$cid}','{$cfilename2}');\">delete</span> | <span class='mod' onclick='modFile($cid)'>modify</span></td>
    </tr>";

Jquery:

function deleteFile(fileId,fileName){
    var fileId = fileId;
    var fileName = fileName;
    var test = this;
    //alert(test);
    $(this).closest('tr').addClass("highlight");
    $('#submit').prop('disabled', true);
    $('#rmDeleteOverlay').fadeIn();
    $('#deleteFile').append("<br><br><center>"+fileName+"</center>");
}

CSS:

.highlight{
    background-color:#000;
}

Any help is appreciated.

function deleteFile(fileId,fileName){
 var fileId = fileId;
 var fileName = fileName;
 var test = this;
 //alert(test);
 $(this).closest('tr').addClass("highlight");
 $('#submit').prop('disabled', true);
 $('#rmDeleteOverlay').fadeIn();
 $('#deleteFile').append("<br><br><center>"+fileName+"</center>");
}
.highlight{
 background-color:#000;
 
}
.mod{
 cursor:pointer;
 color:#a45127;
}
.mod:hover{
 cursor:pointer;
 text-decoration: underline;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<tr bgcolor='#f2e2f2' onmouseover=style.backgroundColor='#FFFFFF'; onmouseout=style.backgroundColor='#f2e2f2';>
   <td><select name='type'><option value='spec'>SPEC</option><option selected value='clg'>CLG</option><option value='coa'>COA</option><option value='gmo'>GMO</option><option value='allergen'>ALLERGEN</option><option value='audit'>AUDIT</option><option value='cor'>COR</option><option value='organic'>ORGANIC</option><option value='kosher'>KOSHER</option><option value='oth'>OTHER</option></select></td>
   <td><a href='/uploads/rm/46/46_clg_20190620223443.pdf' target='_blank'>46_clg_20190620223443.pdf</a></td>
   
   <td><select name='cat'><option selected value='cur'>Current</option><option value='oth'>Other</option><option value='arc'>Archive</option></select></td>
   <td>2019-06-20 22:34:43</td>
   <td><a href='editUser.php?id=11'>11</a></td>
   <td><span class='mod' onclick="deleteFile('1','<a href=\'/uploads/rm/46/46_clg_20190620223443.pdf\' target=\'_blank\'>46_clg_20190620223443.pdf</a>');">delete</span> | <span class='mod' onclick='modFile(1)'>modify</span></td>
  </tr>
connexo
  • 53,704
  • 14
  • 91
  • 128
Sabyre
  • 97
  • 10
  • Can you give an example of the *rendered* HTML, ideally in a runnable Stack Snippet (not in PHP, which we can't run, since we don't have access to your full source code), so we can see the error for ourselves? – CertainPerformance Jun 27 '19 at 21:49
  • 2
    Don't use inline event handlers. Bind properly with `.on()` – j08691 Jun 27 '19 at 21:53
  • 1
    @CertainPerformance Added snippet. – Sabyre Jun 27 '19 at 22:02
  • 2
    When you call a function from `onclick`, it doesn't set `this`. You need to pass the element explicitly. – Barmar Jun 27 '19 at 22:03
  • @j08691 It may be off topic, but can you explain why? I have done some research on inline vs bind and all I could find was "it is frowned upon". Seems to me it is 6 in one half dozen another. Again, I am not strong with jquery. – Sabyre Jun 27 '19 at 22:23
  • (OffT) The `
    ` tag is obsolete - you should use CSS instead. Same goes for your inline JS `onmouse*` Again, use `:hover` instead. It's commn sometimes to overthink a task and overcomplicate it. And don't use HTML-inline `onclick` as well. Use `.addEventListener('click', someFn)` instead.
     – Roko C. Buljan Jun 27 '19 at 22:55
  • @RokoC.Buljan I use CSS 90% of the time falling back to simple HTML when script generated without need for major styling. Rudimentary, but still functional. – Sabyre Jun 27 '19 at 23:08
  • 1
    @Sabyre stop doing it now. Thank me later. ;) :D – Roko C. Buljan Jun 27 '19 at 23:14
  • @RokoC.Buljan Shouldn't the same go for my ``'s? =) Shouldn't I be using `display:grid;` or `display:flex`? Again, I usually do except when pulling data from a DB. Old habits man....
     – Sabyre Jun 27 '19 at 23:22
  • @Sabyre no. Semantics. Here's a nice read... https://www.accessibility-developer-guide.com/examples/tables/table-of-divs-experiment/ – Roko C. Buljan Jun 27 '19 at 23:27

4 Answers4

1

When you call a function like onclick="deleteFile('{$cid}','{$cfilename2}');, it doesn't set this to the element in the function. You need to pass the element explicitly, as onclick="deleteFile(this,'{$cid}','{$cfilename2}');.

You also weren't seeing the style change from the highlight class because the style="background-color: #f2e2f2" took precedence. I moved that into the CSS and made the style for .highlight more specific. Also, use :hover in the CSS instead of onmouseover.

function deleteFile(element, fileId, fileName) {
  $(element).closest('tr').addClass("highlight");
  $('#submit').prop('disabled', true);
  $('#rmDeleteOverlay').fadeIn();
  $('#deleteFile').append("<br><br><center>" + fileName + "</center>");
}
tr {
  background-color: #f2e2f2;
}

tr:hover {
  background-color: #FFFFFF;
}

tr.highlight {
  background-color: #000;
}

.mod {
  cursor: pointer;
  color: #a45127;
}

.mod:hover {
  cursor: pointer;
  text-decoration: underline;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<table>
  <tr>
    <td>
      <select name='type'>
        <option value='spec'>SPEC</option>
        <option selected value='clg'>CLG</option>
        <option value='coa'>COA</option>
        <option value='gmo'>GMO</option>
        <option value='allergen'>ALLERGEN</option>
        <option value='audit'>AUDIT</option>
        <option value='cor'>COR</option>
        <option value='organic'>ORGANIC</option>
        <option value='kosher'>KOSHER</option>
        <option value='oth'>OTHER</option>
      </select>
    </td>
    <td><a href='/uploads/rm/46/46_clg_20190620223443.pdf' target='_blank'>46_clg_20190620223443.pdf</a></td>

    <td>
      <select name='cat'>
        <option selected value='cur'>Current</option>
        <option value='oth'>Other</option>
        <option value='arc'>Archive</option>
      </select>
    </td>
    <td>2019-06-20 22:34:43</td>
    <td><a href='editUser.php?id=11'>11</a></td>
    <td><span class='mod' onclick="deleteFile(this, '1','<a href=\'/uploads/rm/46/46_clg_20190620223443.pdf\' target=\'_blank\'>46_clg_20190620223443.pdf</a>');">delete</span> | <span class='mod' onclick='modFile(1)'>modify</span></td>
  </tr>
</table>

There's also no need for things like

var fileId = fileId;

Function parameters are already local variables, you don't need to re-declare them.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Your answer makes a ton a sense however it still is not working. Again, no errors, no results. – Sabyre Jun 27 '19 at 22:16
  • I have to leave for an hour, I'll implement it in your snippet when I return – Barmar Jun 27 '19 at 22:19
  • @Sabyre I've updated my answer. You need to change how you're setting the background color styles, because the inline style was overriding the CSS. – Barmar Jun 27 '19 at 23:47
  • After I removed the `onMouseOver` & `onMouseOut` from the `` and added `this` to the call and `element` to the function it works flawlessly. See the comment on @sscotti's answer. I now understand that the element needs to be passed to the function to use. I also understand that the duplicate declaration was ridiculous. I still have the `bgcolor=''` in the `` It seems like the `onMouse*=style.backgroundColor=''` was breaking the `` tag. – Sabyre Jun 28 '19 at 00:08
1

To demonstrate what @Barmar already stated, here's code proof:

function foo(context) {
  context.closest('div').style.backgroundColor = 'yellow';
}
<div>Lorem ipsum dolor sit amet consectetur adipisicing elit. Possimus omnis et dolorem nobis?
  <button onclick="foo()">click me</button> Harum, aspernatur molestias at asperiores veniam sit pariatur aut animi, dignissimos consequatur quam quo earum dolor quod?</div>

<div>Lorem ipsum dolor sit amet consectetur adipisicing elit. Possimus omnis et dolorem nobis?
  <button onclick="foo(this)">click me, this time passing context</button> Harum, aspernatur molestias at asperiores veniam sit pariatur aut animi, dignissimos consequatur quam quo earum dolor quod?</div>

If you want to use this, you need to use addEventListener (which you should do anyway, inline event listeners are really bad:

function foo() {
  this.closest('div').style.backgroundColor = 'yellow';
}

baz.addEventListener('click', foo);
<div>Lorem ipsum dolor sit amet consectetur adipisicing elit. Possimus omnis et dolorem nobis?
  <button id="baz">click me</button> Harum, aspernatur molestias at asperiores veniam sit pariatur aut animi, dignissimos consequatur quam quo earum dolor quod?</div>
connexo
  • 53,704
  • 14
  • 91
  • 128
1

Not sure if this is the issue, but in JSFiddle(and elsewhere you might have to have the tr enclosed by table tags. That seemed to be a problem for me.

<!DOCTYPE html>
<html>
<table>
<tr id = "rowelement">
  <td><select name='type'>
  <option value='spec'>SPEC</option>
  <option selected value='clg'>CLG</option>
  <option value='coa'>COA</option>
  <option value='gmo'>GMO</option>
  <option value='allergen'>ALLERGEN</option>
  <option value='audit'>AUDIT</option>
  <option value='cor'>COR</option>
  <option value='organic'>ORGANIC</option>
  <option value='kosher'>KOSHER</option>
  <option value='oth'>OTHER</option>
  </select>
  </td>
    <td><a href='/uploads/rm/46/46_clg_20190620223443.pdf' target='_blank'>46_clg_20190620223443.pdf</a></td>

<td>
<select name='cat'>
<option selected value='cur'>Current</option>
<option value='oth'>Other</option>
<option value='arc'>Archive</option>
</select>
</td>

<td>2019-06-20 22:34:43</td>
<td><a href='editUser.php?id=11'>11</a></td>
<td><span id ="delete" class='mod' data-file = "1" data-filepath = "/uploads/rm/46/46_clg_20190620223443.pdf">delete</span> | <span id ="mod" class='mod' onclick='modFile($cid)'>modify</span>
</td>
</tr>
</table>
</html>

Not sure if the JS works for you, but if you can put the params for the function in data attributes, that would be one method.

$("#rowelement").on("mouseover", function(e) {
    $(this).css("background", "#FFFFFF");
});
$("#rowelement").on("mouseout", function(e) {
    $(this).css("background", "#f2e2f2");
});

$("#delete").on("click", function(e) {
e.preventDefault();
    var fileId = $(this).data("file");
    var fileName = $(this).data("filepath");
    $(this).closest('tr').addClass("highlight");
    // ??? $('#submit').prop('disabled', true);
    // ??? $('#rmDeleteOverlay').fadeIn();
    // ???$('#deleteFile').append("<br><br><center>"+fileName+"</center>");
});

CSS

.highlight{
    background-color: #000 !important;
}
.mod{
    cursor: pointer;
    color: #a45127;
}
.mod:hover{
    cursor: pointer;
    text-decoration: underline;
}
#rowelement {
  background: #f2e2f2;
}
SScotti
  • 2,158
  • 4
  • 23
  • 41
  • Removing the `onMouse*` events from the `` and incorporating the information that @Barmar provided worked. Though I am not sure why. – Sabyre Jun 27 '19 at 23:01
0

Thanks guys for all the information! Especially @Barmar & @sscotti

So as it turns out the issue was 3 fold.

  1. Passing the element info to the function and the use of this. When calling the function behind the click event onClick="deleteFile()" I was trying to get the function to reference the clicked element and ignorantly assumed that element info was automatically passed. @Barmar pointed out that I needed to include this as a parameter in the function call.

  2. The syntax I was using originally in the <tr> for the onMouse* events was incorrect. I was using onMouseOver=style.backgroundColor='#FFFFFF'; This was breaking the <tr> tag. I should have noticed this in jsFiddle, but I didn't. Thank you @sscotti for pointing that out. The line should have been onMouseOver="style.backgroundColor='#FFFFFF';" and similar for onMouseOut.

  3. Further research based on @Barmar mentioning inline style overwriting the CSS lead me to CSS Specificity. Specificity is a basic browser rule with a point system. Where the victor will be the one with the most points. Inline style = 1000, id = 100, class = 10, element = 1.

Using the onMouse* events to change style was equal to 1000 points. I was tring to change that same style in my function by issuing an addClass() which is worth 10 points. The inline style declarations trumped that call, thus the class was ignored.

By removing the inline onMouse* style declarations from the <tr> and moving those to a css class it demoted them from worth 1000 points to being worth 10. Since they are now the same value as the addClass() another basic browser rule is used. When points match, last wins.

Thanks everyone who contributed and helped lead me to the answer. Yes I know some of my scripting is archaic and rudimentary. Perfection is a human bogeyman, IMO it doesn't exist. I live by function before form, but I love improving my practices.

Hope this helps someone.

Sabyre
  • 97
  • 10