jQuery Forum

I'm working on a solution and I have several points about my design that need discussion and feedbacks:

    1- In my proposal the animations (to show or hide the panels of the accordion)  are using the jquery.effects :
        * .animate() method with easing parameters
        or
        * .toggle() method with effect parameters
      In this case, the animations in the current accordion file have been removed and they should be simulated by the standard effect library.

    2- The animation code has been moved into a new widget jquery.ui.panelanimated.js.
        The accordion can work with or without the animated panels.
        The animation of a panel can be used by a stand alone panel with special show-hide effects.
        The accordion can be derived from panel or from panelanimated
   
    3- The event functionality (replace click by mouseover for instance) has been moved into panel.
   
    4- The code to stop any click event before the end of the animation has been moved into panel

My current code is available in this commit :
    http://github.com/dominiquevincent/jquery-ui/commit/1730e838ef8e322a6234e9c31d85c1f7a6e2fb88

All your comments are welcome.

Regards,
Dominique VINCENT

First of all, thanks for the work you've put in here, Dominique! Its definitely useful as a prototype, as it highlights  the problems we have to deal with.

The biggest problem definitely are animations. Animations are the biggest reason for accordion to exist in the first place, and in your fork removed to much of that. Just compare your version with the reference: http://view.jqueryui.com/master/tests/visual/accordion/accordion.html
The big annoying issue of one animation effect other parts of the accordion and the page is back and pretty ugly. Thats the reason for the ton of animation code in the accordion widget, which doesn't even work perfectly in all the various configurations and contexts.

The other highlighted problem: If you ignore animations, event handling and icons are what make up the accordion. In your prototype, _createIcons() and _destroyIcons are reused fully (great!), while only a small part of the event handling is reused - and that affects only about 6 lines of codes, which makes it actually harder to understand whats going on in the (rather obfuscated) event handling code. Certainly not your fault, as the accordion code is already problematic.

Then there's the stuff that isn't in panel yet: ARIA and keyboard handling. It would be interesting to get those into panel, while it doesn't matter so much if we can get them out of accordion (and reused) or not.
Looking at accordion, it probably makes more sense to get panel up and running and released, and deal with accordion later.

So, what I'd like to see: Get rid of panelanimated.js and integrate the old animations back into accordion. Add ARIA and keyboard event handling to panel and try to reuse that in accordion (and maybe more event handling like focus/blur/mouseover/out on headers).

What do you think?

I made the changes ( http://github.com/dominiquevincent/jquery-ui/tree/panel ).
I tried to improve the tabIndex process (it takes into account the original tabIndex of the panel or
accordion). In the previous version, the tabIndex is always set to 0.
I tested the code using Firefox, using the visual tests.
I found 2 differences that require a fix (the fillspace and autoHeight options are not applied on the first panel ...).

Todo list  :
      - fix the problems with options fillspace and autoHeight ,
      - change the unit test files,
      - change the static test files,
      - test with other browsers
      - ? animation in panel (with synchronized support) ?

Send me your feedback and prioritized the next tasks to be done.

Regards,
Dominique

Did you rebase your commit? I still see only one commit... I don't think you should rebase after publishing your history, makes it hard to use your fork as a remote branch. Also makes it hard to see what you changed between your first and second version.

I'll try to actual review the changes and then get back to you.

I used the GitGui option "Amend Last Commit", so you have only one commit. The 2 versions have really nothing in common due to the panelanimated script file. The comparaison should be made with the original panel branch.

I started to work on the test Unit, and I already have some minor changes to done, so it's up to you to wait or not for your review. For the next version, I will make a new commit.

Ah, thanks for the update. You're right, made more sense to replace the old commit.