1055830 - mozilla-accordion.js improvement considerations

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Jonathan Petto [:jpetto]]

While writing docs for mozilla-accordion.js (bug 1007717), Alex Gibson and I had a short discussion regarding possible improvements to the library.

Currently, mozilla-accordion.js relies upon bedrock's global.js file being present (specifically, the gaTrack and trans functions). This doesn't pose an immediate problem, but does affect (potential) use of the library outside of bedrock.

One suggestion made was to add callback support for accordion actions (open/close), leaving it to the specific page implementations to perform GA tracking and/or other required actions. So far, we haven't needed these callbacks, but it's easy to imagine they'd come in handy in the future.

This change wouldn't be too time consuming, but would add complexity. For example, GA tracking, which is now automatic & uniform for all accordions, would need to be added to each implementation. Also, if callbacks were needed, the developer would need to instantiate accordions manually (as opposed to letting the library auto-instantiate).

This bug is to discuss if having a zero-dependency library is worth the added complexity.

Comment 1

[Alex Gibson [:agibson]]

I'd say it's probably not worth the effort of implementing callbacks and needing individual pages handle GA tracking for accordion changes. It would be a much smaller code change to use a type check on the gaTrack function in mozilla-accordion.js. This would probably only be a couple of lines of code, and it could work in a similar way to how we have GA tracking in Tabzilla, but don't have GA as a dependency.

That said, I think adding callbacks to mozilla-accordion.js could be very useful for other things. It might also be nice to be able to create/destroy accordions on demand, and to bring the lib up to parity with mozilla.pager.js in terms of functionality. A suite of JS tests would also be a good addition.

Comment 2

[Jonathan Petto [:jpetto]]

Totally agree with adding a type check to gaTrack and tests. That'd be pretty quick, and wouldn't add complexity.

As for callbacks and programmatic creation/destruction of accordions, I'm torn. Do we increase the size/complexity of the library before there's an actual need for these? Has there been no demand for these features because they don't exist?

I suppose I'm leaning towards adding these features sooner rather than later. Once compressed, the library wont be *too* much heavier with the added code. Our not-too-distant future selves will surely be thankful for the groundwork.

Comment 3

[Alex Gibson [:agibson]]

I think these features all fall in the nice-to-have category. We have no immediate need for them in terms of web pages or projects, but I'd say these are all pretty common methods in modern libs today and will likely come in useful. I think adding them wouldn't add too much to the file size, so I'm not too concerned there. Agree we should probably do this sooner rather than later if we have time available.

Comment 4

[Jonathan Petto [:jpetto]]

I'd say we're agreed on the following:

- Add open/close callbacks
- De-couple from global.js (gaTrack and trans)
- Add ability to programmatically create/destroy accordions
- Add tests
- Update docs

Comment 5

[[github robot]]

Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/0fff95a28a654a6f00fcbbd92d5434aa068dd932
Improve mozilla-accordion.js. Bug 1055830.

- Add collapse/expand callbacks.
- Add ability to programmatically create/destroy accordions.
- Add mozilla-accordion.js tests.
- Add standard GA callback JS for existing accordion pages.
- Standardize mozilla-pager.js create param to jQuery obj.
- Update accordion & pager docs.

https://github.com/mozilla/bedrock/commit/e01e63bc4496d44dc360ea4d047e915a7a124ee9
Merge pull request #2271 from jpetto/bug-1055830-mozilla-accordion-improvements

Improve mozilla-accordion.js. Bug 1055830.