CODEWITHSUNDEEP
javascriptregex
lukeseager
lukeseager

Reputation: 2615

Regex works only once

I've recently been learning JavaScript by creating a little to do list web app here.

So far almost everything's working, but I have an issue if you try to check and uncheck an item more than once. If you keep checking/unchecking you'll see the delete button disappear and --> appear after the urgency icon.

The change of icon is done by a Regex changing code from commented to un-commented. I just don't understand why if it works once, it doesn't work every time?

if (tr.outerHTML.indexOf("checked=\"\"") >= 0) {

    // replace checked with unchecked
    var cookieHTML = tr.outerHTML.replace(/checked=\"\" class=\"list-checkbox\"/, 'class=\"list-checkbox\"')
    .replace(/<tr class=\"list-row done\"/, '<tr class=\"list-row\"')


    // change delete button to urgency.
    .replace(/<!--<span aria-hidden=\"true\" data-icon=\"c\"/, '<span aria-hidden="true" data-icon="c"')
    .replace(/alt=\"Neutral\"><\/span>-->/, 'alt="Neutral"></span>')

    .replace(/<!--<span aria-hidden=\"true\" data-icon=\"f\"/, '<span aria-hidden="true" data-icon="f"')
    .replace(/alt=\"Urgent\"><\/span>-->/, 'alt="Urgent"></span>')

    .replace(/<span aria-hidden=\"true\" data-icon=\"e\"/, '<!--<span aria-hidden="true" data-icon="e"')
    .replace(/onclick=\"deletetodo\(this\)\"><\/span>/, 'onclick="deletetodo(this)"></span>-->');

} else {

    // else add checked to the input.
    var cookieHTML = tr.outerHTML.replace(/class=\"list-checkbox\"/, 'checked class=\"list-checkbox\"')
    .replace(/<tr class=\"list-row\"/, '<tr class=\"list-row done\"')


    // change urgency to delete button.
    .replace(/<span aria-hidden=\"true\" data-icon=\"c\"/, '<!--<span aria-hidden="true" data-icon="c"')
    .replace(/alt=\"Neutral\"><\/span>/, 'alt="Neutral"></span>-->')

    .replace(/<span aria-hidden=\"true\" data-icon=\"f\"/, '<!--<span aria-hidden="true" data-icon="f"')
    .replace(/alt=\"Urgent\"><\/span>/, 'alt="Urgent"></span>-->')


    .replace(/<!--<span aria-hidden='true' data-icon='e'/, '<span aria-hidden="true" data-icon="e"')
    .replace(/onclick='deletetodo\(this\)'><\/span>-->/, 'onclick="deletetodo(this)"></span>');

}

This is the (rather large!) chunk of JS that controls this. Any ideas what's wrong? Or maybe a better way of changing these icons around?

Thanks!

Upvotes: 2

Views: 362

Answers (4)

happybuddha
happybuddha

Reputation: 1358

If all you are trying to do is change icons based on a checkbox being checked or no, you could do something like this. 

 function getVisibility()
{
  var temp = document.getElementById("iconName").style.visibility;

  return temp;
}

function switchIfChecked()
{

  var current = getStyle();

  if( current == "visible" )
   {
     document.getElementById("iconName").style.visibility = "hidden";
   }
   else
   {
     document.getElementById("iconName").style.visibility = "visible";
   }
}

<div id="iconName" style="visibility: visible">INSERT ICON IMG here</div>

What the above does is that it makes the div of the icon visible or hidden. Ofcourse you will need to have two divs and then set either or to hidden or visible. With what you are doing currently, you are not really making the browser do anything.

Upvotes: 1

Amy Palamountain
Amy Palamountain

Reputation: 83

That's awesome that you are learning JavaScript. Nice job. But, I'm quite glad that you posted this question as it looks like you could use a couple of pointers.

The answer to your question is - yes there is a much simpler way to achive this effect - which I will get to shortly. But first - I notice that at the bottom of your todo app you include a library called JQuery

 <script src="//ajax.googleapis.com/ajax/libs/jquery/1.8.3/jquery.min.js"></script>

This library will be of huge help to you, not only in the function you describe above, but to the majority of the code you have written. You will end up with much cleaner and self explanatory code.

http://jquery.com/

Basically what JQuery allows you to do, is to manipulate the state of the DOM. You definatly want to begin here.

Here is small sample which shows a check box who can be checked or unchecked, and on change, have an element shown' or hidden as desired.

http://jsfiddle.net/m4vGE/5/

Please - do take the time to have a look into JQuery - its a great first step you can take to increase your produtivity and reduce complexity in your JavaScript

Also - as a side note, if you find yourself using js to build HTML with strings, the answer is invariably "there is a better way"

Upvotes: 2

PeeHaa
PeeHaa

Reputation: 72672

I would say: you're are doing it wrong. Using a string / regex replacement method is not the right way to go imho.

Instead of doing those replacement use DOM methods, i.e.:

someElement.setAttribute('data-icon', 'f');
someElement.setAttribute('alt', 'Urgent');

A simple example can be found here: http://jsbin.com/iwakof/1/edit

I know this isn't a direct answer to your question, but trust me this is the way to go

Upvotes: 4

Sebastian vom Meer
Sebastian vom Meer

Reputation: 5241

Try to use the global option of Regex (have a look at the g after the second slash):

// ...
.replace(/<tr class=\"list-row done\"/g, '<tr class=\"list-row\"')
// ...

Here is an example.

From developer.mozilla.org:

global: Whether to test the regular expression against all possible matches in a string, or only against the first.

Upvotes: 0

Related Questions