5

I'm trying to understand how to handle some events with javascript, in particular if a specific event is triggered the current function calls another function. At onLoad the javascript creates the first select the data from the first object. Then if I select the voice with value "Iphone" is called another function for create the second menu with some prices. My main problem is that my solution seems quite ugly, and also when i click more than one time on the button it creates every times the same second select. 

//global objects
var product = [
  {name: "Samsung"},
  {name: "Iphone"},
  {name: "Alcatel"},
  {name: "Sony"}
]

var productPrice = [
  {name: "Samsung", price: 190},
  {name: "Iphone", price: 290},
  {name: "Alcatel", price: 65},
  {name: "Sony", price: 330}
]

var main = function() {
  var sel = $('<select>').appendTo('#select-1');
  
  $(product).each(function() {
    sel.append($("<option>").attr('value', this.name).text(this.name));
  });
  
  $('.press').click(function() {
    if ($("#select-1 option:selected").text() == "Iphone") {
      handleEvent();
    }
    
    if ($("#select-1 option:selected").text() != "Iphone") {
      $("#risposta").remove();
    }
  });
}

function handleEvent() {
  var sel = $('<select>').appendTo('#select-2');
  
  $(productPrice).each(function() {
    sel.append($("<option>").attr('value', this.name).text(this.price));
  });
}

$(document).ready(function() {
  main();
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div id="select-1">
</div>
<div id="select-2">
</div>
<button class="press">Click</button>

fiddle

Any suggestions?

Improve this question
7
  • the reason that it created another element is that handleEvent() function is creating an element. Commented Sep 10, 2016 at 15:46
  • and to continue on @JericCruz's point, you basically have two options: 1) remove the list always before appending 2) toggle visibility (eg. css class) instead of append Commented Sep 10, 2016 at 15:48
  • @Jeric Cruz I know, cause the function that I have declared creates every time the second select. I have imagined something like this code for handle this event, but it seems not worth. How can I organize better my functions for avoid these events problems? I mean for call just one time the function that creates the second select, and of course remove that when i click on other select values. Commented Sep 10, 2016 at 15:53
  • $("#select-1 option:selected") probably needs to be $("#select-1").find("select option:selected") Commented Sep 10, 2016 at 15:53
  • Something like this? jsfiddle.net/qu9zs2h1 , Just make sure the select isn't already created, just use a class or an id to make sure it hasn't already been appended Commented Sep 10, 2016 at 15:59

2 Answers 2

Reset to default
2

You question is not 100% clear to me however I gather you want to generate a select tag on dom ready event which you have already. When you select the option iPhone and hit the Click button you want to generate another select tag with the new options from your variable.

First option

Is what I have provided below. Its basically your code but with an extra if statement

jsFiddle link : https://jsfiddle.net/qu9zs2h1/2/

Javascript

//global objects
var product = [{
  name: "Samsung"
}, {
  name: "Iphone"
}, {
  name: "Alcatel"
}, {
  name: "Sony"
}]

var productPrice = [{
  name: "Samsung",
  price: 190
}, {
  name: "Iphone",
  price: 290
}, {
  name: "Alcatel",
  price: 65
}, {
  name: "Sony",
  price: 330
}]

var main = function() {
  var sel = $('<select>').appendTo('#select-1');

  $(product).each(function() {
    sel.append($("<option>").attr('value', this.name).text(this.name));
  });

  $('.press').click(function() {
    if ($("#select-1 option:selected").text() == "Iphone") {
      handleEvent();
    }

    if ($("#select-1 option:selected").text() != "Iphone") {
      $("#risposta").remove();
    }
  });
}

function handleEvent() {
  // We still run the handleEvent function however, if check to see
  // if the second select option has been generated by checking a 
  // class
  if (!$('.prices').length) {
    var sel = $('<select class="prices">').appendTo('#select-2');
    $(productPrice).each(function() {
      sel.append($("<option>").attr('value', this.name).text(this.price));
    });
  }
}

$(document).ready(function() {
  main();
});

Html

<div id="select-1">
</div>
<div id="select-2">
</div>
<button class="press">Click</button>

I had to add the class prices to your second select but that is just a class you can rename which is used to see if that element already exists.

The second option (Probably the better way)

Is you generate everything on dom ready and hide what you want at the start and then simply show elements when you want, like so

jsFiddle : https://jsfiddle.net/qu9zs2h1/5/

Javascript

//global objects
var product = [{
  name: "Samsung"
}, {
  name: "Iphone"
}, {
  name: "Alcatel"
}, {
  name: "Sony"
}]

var productPrice = [{
  name: "Samsung",
  price: 190
}, {
  name: "Iphone",
  price: 290
}, {
  name: "Alcatel",
  price: 65
}, {
  name: "Sony",
  price: 330
}]

// On document ready run this script
$(function() {
    $('.jshidden').removeClass('jshidden').hide();

  var $select1 = $('#select-1');
  var $select2 = $('#select-2');
  var $sel1 = $('<select>').appendTo($select1);
  var $sel2 = $('<select>').appendTo($select2);

  $(product).each(function() {
    $sel1.append($("<option>").attr('value', this.name).text(this.name));
  });

  $(productPrice).each(function() {
    $sel2.append($("<option>").attr('value', this.price).text(this.price));
  });

  $('.press').click(function() {
    if ($select1.find(":selected").text() == "Iphone") {
      $select2.show();
    }

    if ($select1.find(":selected").text() != "Iphone") {
      $select2.hide();
    }
  });
});

Html

<div id="select-1">
</div>
<div id="select-2" class="jshidden">
</div>
<button class="press">Click</button>

P.S. just a little something, anything which is referring to a variable which is a jQuery element should start with a $ it just helps read the code.

Just a quick update, you can also reduce the .press code with this code

$('.press').click(function() {
  var iPhoneCheck = $select1.find(":selected").text() == "Iphone";
    $select2.toggle(iPhoneCheck);
  });

jQuery.toggle takes in a bool value to either display the target or hide the target

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Thank you @Canvas, both solutions are nice! So in the second version you have simplified and replaced the call function with more specific if statements. In your opinion which is the best choice as code structure?
The second option would be the best one as you don't modify the dom again. Also if any of the provided answers are correct make sure to tick the answer
0 

//global objects
var product = [
  {name: "Samsung"},
  {name: "Iphone"},
  {name: "Alcatel"},
  {name: "Sony"}
];

var productPrice = [
  {name: "Samsung", price: 190},
  {name: "Iphone", price: 290},
  {name: "Alcatel", price: 65},
  {name: "Sony", price: 330}
];

var main = function() {
  var sel = $('<select>').appendTo('#select-1');
  $(product).each(function() {
    sel.append($("<option>").attr('value', this.name).text(this.name));
  });
  
  $('.press').click(function() {
    var sel2 = document.getElementById('select-2');
    var selectedOption = $("#select-1 option:selected").text();
    if (selectedOption == "Iphone") {
      // check if the select was already added
      if(sel2.children.length == 0)
        handleEvent();
    }
    else {
      // I think here you wanted to remove the second select that was added for iphone option
      sel2.innerHTML = '';
    }
  });
}

function handleEvent() {
  var sel = $('<select>').appendTo('#select-2');
  $(productPrice).each(function() {
    sel.append($("<option>").attr('value', this.name).text(this.price));
  });
}

// if you already have a declared function, there is no need to an anonymous function
// just call the main function directly
$(document).ready(main);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div id="select-1">
</div>
<div id="select-2">
</div>
<button class="press">Click</button>

Improve this answer

5 Comments

Is there any reason you are missing javascript and jquery dom selectors? you can just use $, also removing the select and re-appending it is silly to do, you should just check the length and if it exists do nothing
I just hadn't realized i was mixing the two selectors, just didn't pay attention, no special reason. I removed the select appended because I assumed that when another option was selected in the first select (samsung, acatel, sony), the appended select should be removed as in the snippet of the question there was a line of code trying to do something like that. ($("#risposta").remove();)
Okay, I have realized what I missed as logic: i forgot to make a correct if statement for add/remove the second select. So you have declared sel2 and you have controlled if #select-2 empty with with his children.length. That's nice! Any suggestions also for improve and minimize the current code? Thank you again guys!
There is something I didn't understand. All that prices that are being appended to when I choose "Iphone" (190, 290, 65, 330). Are they really supposed to be added? Or just 290 that is the price for Iphone? One suggestion is that you don't need the two objects: product and productPrice. "productPrice" already have all you need. If you really need to append all the prices as i am asking, you just use an array in the price property of the productPrice object.
Thank you for the advices @jplobianco i'll use just one object so.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.