badger-accordion icon indicating copy to clipboard operation
badger-accordion copied to clipboard

Renaming header to trigger

Open stuartjnelson opened this issue 6 years ago • 2 comments

Currently, the <button> to open a panel is called header. I think this could be confused with the HTML element and should be renamed to trigger in the docs as well as any other properties (class, data-attr etc...).

@seanjhulse what do you think? I can't workout a clean way to do this without creating a breaking change. Any ideas/opinions would be most welcome!

stuartjnelson avatar Aug 19 '18 15:08 stuartjnelson

I can't think of any way to avoid breaking existing projects. We could provide a way to pass in a custom "header/trigger" CSS class during instantiation. The change would still break existing projects, but it would allow for people to easily incorporate the previous CSS class of .js-badger-accordion-header to accommodate the older version.

seanjhulse avatar Aug 20 '18 14:08 seanjhulse

Thanks for your opinion @seanjhulse!

I have an option for the class so that's covered. I haven't done an install option before but that would be cool. I wonder if I should add in an install warning message for version 2.0.0.

There are only two methods that would be changed and that could be depreciated, both of which are "private".

_openHeadersOnLoad()
_setupHeaders()

I guess the old options that use the word header in could use the value of trigger. This way thFor example

Current option

headerDataAttr: 'data-badger-accordion-header-id',

New option

triggerDataAttr: 'data-badger-accordion-trigger-id',

get headerDataAttr() {
    return this.triggerDataAttr;
},

Lastly, the docs would need updating. I wonder if the easiest thing to do would be to provide a new documentation file and some how archive the old one. Have you seen this done before or done it yourself? I couldn't find anything form my quick search but I wonder if version 1.x.x has a file readme-version-1.md then readme.md is docs for version 2.x.x. Does that make sense? Is there a better way to do it?

stuartjnelson avatar Aug 22 '18 15:08 stuartjnelson