CODEWITHSUNDEEP
javascriptjqueryhtmlcss
Jedis
Jedis

Reputation: 21

Moving inline javascript function to external file and convert to jQuery

I am trying to optimize my "spoiler" bbcode on phpBB3.

Right now, I have a working solution, but the inline javascript is injected by phpBB every time the "spoiler" bbcode tag is used. I want to call a common function instead of adding it inline every time the bbcode is used.

Here is that working inline javascript:

<div class="spoiler">
    <div class="spoiler-title">
        <span onclick="if (this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display != '') { this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display = ''; this.parentNode.getElementsByTagName('a')[0].innerText = 'hide'; } else { this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display = 'none'; this.parentNode.getElementsByTagName('a')[0].innerText = 'show'; }">
            <strong>{TEXT1}</strong> (<a href="#" class="spoiler-btn" title="Show hidden content">show</a>)
        </span>
    </div>
    <div class="spoiler-text">
        <div style="display: none;">
            {TEXT2}
        </div>
    </div>
</div>

For ease of reading, the inline onclick function is repeated here:

if (this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display != '') {
    this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display = '';
    this.parentNode.getElementsByTagName('a')[0].innerText = 'hide';
} else {
    this.parentNode.parentNode.getElementsByTagName('div')[1].getElementsByTagName('div')[0].style.display = 'none';
    this.parentNode.getElementsByTagName('a')[0].innerText = 'show';
}

Clicking the anchor with the class of "spoiler-btn" has a preventDefaults on it, to prevent the click from taking you to the top of the page:

$(document).ready(function(){

    $(".spoiler-btn").click(
      function(e) {
        e.preventDefault();
      }
    );

});

I was trying to replace the span onclick inline javascript with a function call that passes 'this' to an external javascript file. I couldn't seem to get that working, so I tried using jQuery to capture 'this' to traverse up the DOM to find the "div" contained within the "spoiler-text" div and manipulate the display:none. There can be multiple of these spoiler tags on the page, so I cannot give the div inside of the "spoiler-text" div an id.

Here I changed the onclick of the span to the external function:

onclick="spoilerToggle(this);"

I then have the following in my external file:

var spoilerToggle = function(param) {
    if ($(this).parent('div').parent('div').hasClass('spoiler-text').css('style') == 'none') {
        ($(this).parent('div').parent('div').hasClass('spoiler-text').removeAttr('style'));
        ($(this).parent('div').$('a').text('hide'));
    } else {
        ($(this).parent('div').parent('div').hasClass('spoiler-text').css('display', 'none'));
        ($(this).parent('div').$('a').text('show'));
    }
}

The console then gives the following error: bbcode.js:22 Uncaught TypeError: $(...).parent(...).parent(...).hasClass(...).css is not a function

Line 22 is the line with the "if" check.

jQuery is loaded on the site, and I've made sure to call my external javascript file right before the close of the body tag.

I feel like I've gone down the rabbit hole and cannot see the light. I'm sure this is much easier than I am making it out to be.

Any help is greatly appreciated. Thank you!

Upvotes: 0

Views: 1567

Answers (1)

nnnnnn
nnnnnn

Reputation: 150010

.hasClass() returns a boolean, so you can't chain other methods after it. That's why you get the error you quote.

I would implement it a different way though:

$(document).on("click", ".spoiler-title", function(e) {
  e.preventDefault();
  var container = $(this).closest(".spoiler");
  container.find(".spoiler-btn").text(function(i, currentText) {
    return currentText === "show" ? "hide" : "show"
  });
  container.find(".spoiler-text div").toggle();
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<div class="spoiler">
    <div class="spoiler-title">
        <span>
            <strong>{TEXT1}</strong> (<a href="#" class="spoiler-btn" title="Show hidden content">show</a>)
        </span>
    </div>
    <div class="spoiler-text">
        <div style="display: none;">
            {TEXT2}
        </div>
    </div>
</div>

The above uses a single, delegated click handler bound to the document to handle clicks on all spoiler elements on the page (you could instead bind it to a lower-level container element, at whatever the lowest level is that contains all the spoilers).

Within the handler, this will refer to the clicked element, so with DOM navigation methods such as .closest() and .find() you can go up to the containing div and then down to the elements you want to manipulate. .closest() is more flexible than trying to chain .parent().parent(), because it will automatically go up until it finds an element matching the specified selector, so if you later change your HTML structure the JS probably won't need to change.

If the .text() call looks confusing, what happens there is jQuery calls the function I passed to .text() as an argument, passing it the current value of the element's text and then whatever value is returned becomes the new text.

Upvotes: 2

Related Questions