Fx.Accordion previous index incorrect value in the events #230

Closed
pablocusto opened this Issue Apr 5, 2011 · 1 comment

Comments

Projects
None yet
2 participants

Example:

var index = 0;

var myAccordion = new Fx.Accordion(togglers, elements, {
    onActive: function(toggler, element){
        alert(index===this.previous);
    }
});

myAccordion.display(index);  // return true (index = 0 and this.previous should be -1)
index++;
myAccordion.display(index); // return true (index = 1 and this.previous should be 0)

Suggestion #1:

Set "this.previous" before the last line of "display" method

display: function(index, useFx){
   ..
   ...
   this.previous = index;
   return useFx ? this.start(obj) : this.set(obj);
}

Suggestion #2:

Add "this.current" index value

initialize: function(){
  ..
  ...
  this.current = this.previous = -1;
  ..
},

display: function(index, useFx){
   ..
   ...
   this.current = index;
   this.selfHidden = false;
   ..
   ...
   this.previous = index;
   return useFx ? this.start(obj) : this.set(obj);
},

xergio commented Dec 1, 2011

If you avoid the condition in your example you get an alert for each element in th accordion.

This is my suggestion to fix the problem, in display() method:

1: add an IF:

+++         if (index === i || this.previous === i)
                this.fireEvent(hide ? 'background' : 'active', [this.togglers[i], el]);
  1. To change the this.previous setting position to the end:
+++     this.previous = index;
        return useFx ? this.start(obj) : this.set(obj).internalChain.callChain();
    }

This solve the problem for me.

@SergioCrisostomo SergioCrisostomo added a commit to SergioCrisostomo/mootools-more that referenced this issue May 1, 2014

@SergioCrisostomo SergioCrisostomo fixes #230, this.previous returns wrong index
Accordion's `this.previous`is returning wrong values.
Example (wrong behaviour): http://jsfiddle.net/BgGCN/1/

this.previous is returning the current open, instead of last opened.

New version (correct behaviour): http://jsfiddle.net/BgGCN/
7a08187

@SergioCrisostomo SergioCrisostomo added a commit that referenced this issue May 1, 2014

@SergioCrisostomo SergioCrisostomo Merge: Fix accordion's `this.previous`
Accordion's `this.previous` was returning wrong values.
Example (wrong behaviour): http://jsfiddle.net/9TnkF/

`this.previous` is returning the current open, instead of last opened.

New version (correct behaviour): http://jsfiddle.net/BgGCN/

fixes #230
closes #1255
24f2476
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment