CODEWITHSUNDEEP
javascriptjquery
jdogdvr
jdogdvr

Reputation: 369

A good way to write a jQuery function that sets a global variable for specific events?

I'm using a canvas library to carry out a few drawings with different canvas functions available from the library (I'm using fabric.js).

My javascript skills are not great at all so would appreciate advice on how to structure my code correctly. Please note that this is not a fabric.js issue but simply javascript/jQuery.

I have a few canvas functions but the issue arises when I need a remove object/eraser tool. 

    var isEraserActive = false;

    $('#cursorCv').click(function(){
      canvas.isDrawingMode = false;
      isEraserActive = false;
    });

    $('#eraserCv').click(function(){
      canvas.isDrawingMode = false;
      canvas.deactivateAll().renderAll();
      isEraserActive = true;
    });

    canvas.on('object:selected', function(options){
  		if(isEraserActive){
        canvas.remove(options.target);
      }
    });

I don't like the idea of having to add isEraserActive = false; to every function just so the if(isEraserActive) in the event listener condition satisfies. There has to be a much more efficient way of achieving this. Also if my functions take too long to run and the event listener fires wouldn't the global variable not change in time? It just looks like really bad code structure, for example I will be having much more canvas functions below.

$('#squareCv').click(function(){
   var isEraserActive = false;   
});

$('#hollowSquareCv').click(function(){
   var isEraserActive = false;   
});

$('#circleCv').click(function(){
  var isEraserActive = false;    
});

$('#hollowCircleCv').click(function(){
   var isEraserActive = false;   
});

$('#fontTextCv').click(function(){
   var isEraserActive = false;   
});

What would be the best way to approach this issue?

Thanks!

Upvotes: 0

Views: 73

Answers (5)

JF-Mechs
JF-Mechs

Reputation: 1081

You can set a public variable which you can access across your script for you to stop on re-instantiating the same variable. Also, you can do a delegated click event. In this approach, you don't have to create a lot of click event and this will allow you avoid adding event listeners on specific element.

var isEraserActive = true,
  eraserInActiveList = ['squareCv', 'hollowSquareCv', 'circleCv', 'hollowCircleCv', 'fontTextCv'];

$('#page-content').on('click', function(e) {
  var targetElement = e.target;
  
  if (targetElement && eraserInActiveList.indexOf(targetElement.id) > -1) {
    //For making eraser in active
    isEraserActive = false;
    alert('Eraser active: ' + isEraserActive);
  } else if (targetElement && targetElement.id === 'another-button') {
    //Do something else
    alert('Doing something else');
  } else if (false) {
      //Update with your own condition to meet your needs.
  }
  
});
#canvas-test {
  margin: 16px 12px;
  width: 100%;
  height: 250px;
  border: 1px solid;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<section id="page-content">
  <button type="button" id="squareCv">Square</button>
  <button type="button" id="hollowSquareCv">Hollow Square</button>
  <button type="button" id="circleCv">Circle</button>
  <button type="button" id="hollowCircleCv">Hollow Circle</button>
  <button type="button" id="fontTextCv">Font Text</button>

  <!-- Example content -->
  <canvas id="canvas-test"></canvas>
  <button type="button" id="another-button">Sample Button that do something else</button>
  <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras maximus lacinia ligula, vitae ultrices sem ornare quis. Proin mi arcu, accumsan venenatis ligula eu, molestie laoreet turpis. Proin viverra feugiat dui, sit amet commodo sapien tincidunt
    in. Duis lacus tellus, pulvinar non tellus eget, faucibus aliquam libero. Duis condimentum quam tortor, suscipit molestie magna gravida et. Curabitur vel quam suscipit, condimentum felis ac, scelerisque mauris. Fusce id elit felis. Vestibulum id porta
    eros, efficitur tempus urna. Maecenas pulvinar arcu ut urna scelerisque rhoncus. Donec et ullamcorper dui. Donec non interdum elit. Quisque quis lectus facilisis, ultrices felis ac, rhoncus massa. Sed dictum tempor mi nec consequat. Phasellus pharetra
    ut mi eget pulvinar. Nulla tempor nunc at dolor vestibulum consequat.</p>

  <hr />

  <p>Suspendisse quis felis feugiat, aliquam nulla at, lobortis mi. Nulla mattis vitae ante sed vehicula. Quisque dui justo, ultricies quis iaculis suscipit, scelerisque at erat. Proin rhoncus at mauris eget suscipit. Aliquam interdum sollicitudin posuere.
    Pellentesque hendrerit placerat venenatis. Etiam quis euismod ex, at elementum diam.</p>
</section>

Upvotes: 1

Tomi
Tomi

Reputation: 245

You can bind one handler function to multiple HTML DOMs events. For example:

function setEraserClickHandler(event){
    isEraserActive = false;
}
$('#squareCv').click(setEraserClickHandler);
$('#hollowSquareCv').click(setEraserClickHandler);
$('#circleCv').click(setEraserClickHandler);
$('#hollowCircleCv').click(setEraserClickHandler);
$('#fontTextCv').click(setEraserClickHandler);

Or you can send parameters to the handler too:

function setEraserClickHandler(eraser_active, event){
    isEraserActive = eraser_active;
}
$('#fontTextCv').click(this.setEraserClickHandler.bind(this, false));
$('#hollowSquareCv').click(this.setEraserClickHandler.bind(this, false));

etc..

Or maybe is better to add to all these objects a common class, and then bind the event handler just to that one class.

Upvotes: 0

Ronnie Smith
Ronnie Smith

Reputation: 18575

A Revealing Module Pattern is good practice as your variable is namespaced so it won't conflict with other JS that may be added or running.

var yourAppName = (function() {
    var pub = {};
    pub.eraser = false;
    //API
    return pub;
}());

To set this variable or to check it

yourAppName.eraser //false
yourAppName.eraser = true;
yourAppName.eraser // true

A JavaScript Closure might also be used but is a little more difficult to understand.

Upvotes: 0

jfriend00
jfriend00

Reputation: 707436

Give all your isEraserActive turn off objects a common class name and then use this just once:

$(".eraserOff").click(function() {
    isEraserActive = false;
});

Or, use some other common selector (you'd have to show us your HTML for us to know what to recommend) that automatically includes all the desired elements. You could also use a single parent event handler using delegated event handling that examines what type of object was clicked on to decide whether it should change the eraser or not. There are always lots of options for common event handlers for multiple elements. Which to use depends a lot on your specific HTML which you have not disclosed.

Also if my functions take too long to run and the event listener fires wouldn't the global variable not change in time?

No, this is not a problem. If your functions are taking a long time to run, nothing else is running either so things are still sequenced properly.

Upvotes: 0

Itzik.B
Itzik.B

Reputation: 1076

I dont know how your code look like exactly, but you can declare the var outsite before all the function. instand of declare it in each event. or you can do that like that:

$('#hollowCircleCv,#fontTextCv,#circleCv,#hollowSquareCv,#squareCv').click(function(){
   var isEraserActive = false;   
});

Upvotes: 0

Related Questions