ampersand-dom-bindings icon indicating copy to clipboard operation
ampersand-dom-bindings copied to clipboard

Make bindings extensible

Open RickButler opened this issue 7 years ago • 4 comments

For some background on the issue #53 and AmpersandJS/ampersand-view#178

This should also satisfy #16 and #37

The binding store is a singleton loosely modeled after epoxy. It allows adding custom bindings to Ampersand without overwriting the default bindings and is much more powerful than a custom binding function. I did not change any of the logic inside the default-bindings.

Concerns and Notes: I moved several reusable functions into utils (following what seemed to be the pattern from ampersand-events). I would like to move setAttrbiutes and removeAttributes in ampersand-dom to clean things up. I would then move get-matches into the root as get-matches.js.

Originally I had the ampersand prefix on binding-store and default-bindings, I removed them because it made the file name too long and repetitive if you are accessing them via require e.g. require('ampersand-dom-bindings/ampersand-binding-store') becomes require('ampersand-dom-bindings/binding-store')

RickButler avatar Feb 24 '18 14:02 RickButler

Tagging @cdaringe because of your involvement in the original issue, and @dhritzkiv since you are the most active maintainer.

RickButler avatar Feb 24 '18 14:02 RickButler

Hey @RickButler. Thanks for this comprehensive PR. I think this looks super promising and useful, especially after a little cleanup.

However, seeing as this is a large and fundamental change, I don't feel comfortable reviewing this alone. I'd need commitment from another team member before starting to discuss this further.

dhritzkiv avatar Feb 24 '18 22:02 dhritzkiv

Thanks @dhritzkiv, if you see any style changes, bug, errors, etc. Let me know.

Should I tag anyone else?

RickButler avatar Feb 26 '18 15:02 RickButler

No, I think we're the only two active-ish maintainers, unfortunately. I've added us as reviewers for the PR

dhritzkiv avatar Feb 26 '18 15:02 dhritzkiv