issue/2824-jsx JSX version of accordion

[oliverfoster]

issue/2824-react

part of adaptlearning/adapt_framework#2876

An example of how to create a react version of a component:

1. ViewClass.template = 'name.jsx'; Add .jsx to the end of the template name (Template names are sometimes implied from the Adapt.register call).
2. Convert the hbs template into a jsx template, the jsx template should reflect all of the view states, including any DOM modifications from the javascript.
3. Remove all of the DOM modifications from the javascript (in the case of the accordion below, jquery still handles the animation)

This is called a declarative approach, where we declare the state of the DOM in jsx and allow the computer to figure out how to implement it. We have been using an imperative approach, where we have to tell the computer how to make DOM modifications. Reference on declarative vs imperative

[oliverfoster]

I think we could probably do the event attachment both ways @cahirodoherty-learningpool
I'm not sure which is easier to understand.
Attachment by the callback onClick or attachment where the click happens?

[cahirodoherty-learningpool]

I think we could probably do the event attachment both ways @cahirodoherty-learningpool
I'm not sure which is easier to understand.
Attachment by the callback onClick or attachment where the click happens?

Yeh I'm not sure which is the best way to declare this, but happy with either as long as there is consistency across views.

[oliverfoster]

I think if we step away from backbone for the events that'd be the best long-term plan.

[oliverfoster]

Does it all make sense otherwise?

[cahirodoherty-learningpool]

👀

[NayanKhedkar]

@oliverfoster can we get rid of var most of the(one time assigned) variables to const now
e.g.
var isResetOnRevisit = this.model.get('_isResetOnRevisit');
var index = item.get('_index');

[moloko]

couple of minor suggestions, otherwise looking great. Bring it on!

[oliverfoster]

Do not merge without adaptlearning/adapt_framework#2829 and adaptlearning/adapt_framework#2827 version bump accordingly when ready.

Notes:

• Will need accessibility and general functionality testing.
• May not work with some extensions that inject themselves into the DOM.

[oliverfoster]

@kineojen : could you double check the img loading=eager attribute when you next run this?

[ghost]

@kineojen : could you double check the img loading=eager attribute when you next run this?

Confirmed all accordion images are set to loading="eager"

[cahirodoherty-learningpool]

👀

[oliverfoster]

Ready to review, requires pr#3140