-
Notifications
You must be signed in to change notification settings - Fork 389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for adding plugins #98
Comments
It seems that a plugin system will slightly break current API. That way I would go with a new milestone (v4). If so it would be great to have following features/changes:
And there are few questions:
Other changes to consider for v4 would be:
|
Agreed that this would be a new major version and also on the list of features/changes. Lots of good questions, and I don't have answers to them all at this point.
Not sure about this one, although my comment at the bottom about Highland might be related.
This sounds analogous to
Thinking this through - so a click could trigger new zoom event, and then we would then only use zoom events in the core to initiate zooming...
Following on from the above click example, we could cancel the zoom event, but I'd be hesitant to cancel the click event.
Not sure. jQuery does something where when
If possible, it would be nice to allow for a combination, such as we allow the plugins to listen to raw browser events (but not cancel them) and also subscribe to processed (or wrapped) events. The event handling ideas from Highland.js are pretty interesting. For example: var clicks = _('click', btn).map(1);
var counter = clicks.scan(0, _.add);
counter.each(function (n) {
$('#count').text(n);
}); Then if you want to have multiple listeners, you can fork that event stream. |
Here is a library that has some plugins functionality. Not what we need but can be useful for inspiration.
After some thought a solution that I see is:
We may also have subscription to Also I'm not sure about wrapped events attributes. They may look different. But the idea is that you should have access to original event, and you should be able to alter library default computation results. |
Right, I mentioned Highland for inspiration but don't want to make it a dependency. That microplugin library has some good ideas. Let's go with your proposed solution. Nice work! For the wrapped events attributes, we can mimic jQuery attributes, whenever appropriate, to make it easier for other developers to understand. |
@dlg-yahoo, if you have any preferences here, feel free to chime in! |
I feel like all these ideas are still raw and need some more thought. Also some implementation prototypes would be helpful. Unfortunately right now I don't have time to work on these things actively as I'll be in India this whole month (it really takes time to travel). |
One more thing to think about: How to allow plugins to update SVG manually. For example if you want to animate zoom/pan then you may want inner state of library to have final values, manipulation with SVG done by the plugin. Or maybe such a thing is not necessary and there are other solution that would allow to achieve the same result. Related to #101 |
Yeah, this is an ambitious refactoring, and I won't be able to get to it for awhile either. Let's keep thinking about it and then try some prototypes as time allows. I hadn't thought of the use case from #101, but I can see how that might be useful. If we can accommodate something like that, it would be great, as long as we don't start over-engineering this library to point where it's painful to work with. |
Also, for any changes that we are more sure about, let's not let this issue become a roadblock. We can come back to this at any time. |
I agree on fact that this issue shouldn't be a stopper, we can still add minor versions to v3. |
I did start working on this. There will be few major changes:
|
Add plugins system Add events system Mock middlewares system Replace on/before Pan/Zoom callbacks with events Trigger events for CTM render Rename plublicInstance to publicApi Based on #98
Add plugins system Add events system Mock middlewares system Replace on/before Pan/Zoom callbacks with events Trigger events for CTM render Rename plublicInstance to publicApi Based on #98
Removed all the secondary functionality such as: - Browser events - Enabling/Disabling pan/zoom events - Control icons - Double click - Mousewheel zoom - Min/max zoom - fix/contain/center Removed functionality will be moved into plugins or as easy to implement examples. Browser events (click, mousemove...) will be moved into a plugin. The same goes for mousewheel. Everything will be around events. Some of them will have alterable event data. Related to #98
Public API and each module API methods are namespaced. This means if you call a plugin API method that results into a panzoom event then this event will contain that plugin name as event namespace. Such a behaviour is useful for plugins that want to cancel certain events and trigger those events by themselves without getting into a recursion. One such example is a plugin that will animate pan/zoom. API creation was moved into a module. Reference to inner methods is intentionally hidden so that it is not accessible from exterior. Related to #98
All this changes are available in |
@bumbu, what are your thoughts on SVG animation vs. just pan and zoom? My initial thought for this library was to make it a simple drop-in solution for just pan and zoom. But many users want full animation capabilities. Just brainstorming for What do you think about supporting use with the SVG As you suggest, we could offer multiple builds. Maybe:
|
Hey @ariutta, Regarding animation: I agree that ideally the library would provide the bare minimum (aka pan and zoom) and animations would come as an add-on/extension. As for the future versions ideally we'd want as you mentioned to provide the core/basic manipulations with ability to extend/hook into them with plugins/add-ons. We could bundle them in different versions or provide instructions how to build different bundles but based on current state of JavaScript - most projects have their own bundlers and we'd only need to provide the source. Not sure thought that we want to support the SVG script tag as it feels like an edge case which can be covered by simply embeding the SVG into an HTML page. But starting with a plugin system would be a great start and based on the usage of that it should be easy to see what people need the most (plus we have a ton of suggestions in issues :). As you can see from this thread I started building the plugin system at some point, but never finished it, and I don't have time to do it any time soon. But if anyone wants to work on that then I'd be definitely open to help. |
I'm hoping to find some time at some point here, but it is a big task for a free side project! If I have the chance to dig into this, I'll let you know. |
The core of the library is now nearly feature complete, so to keep the size reasonable, we'll think twice before adding new features to the core.
But sometimes additional features will be useful to a subset of users. To allow for sharing the code for these kinds of features, we need to refactor to make it easy to add plugins. Then we can freely add unlimited new features as plugins while keeping the core of the library as small as possible.
Work in progress
center
,contain
andfit
into a plugincustomEventsHandler
render
event (onZoom, onPan events to early #121)@bumbu and @dlg-yahoo, are there other things needed to make it easy to add plugins? It might be awhile before I have time to do this myself, so don't let me be the bottleneck if either of you want to tackle this sooner. I'm fine with whatever you prefer, @bumbu.
The text was updated successfully, but these errors were encountered: