search

ApryseSDK / webviewer-ui

WebViewer UI built in React
Other
416 stars 355 forks source link

OnAnnotationSelected event is not bubbled up #202

Closed glendaviesnz closed 5 years ago

glendaviesnz commented 5 years ago

We need to be able to detect an OnAnnotationSelected event in our app that wraps webviewer.

Currently there is no $(document).trigger in this event listener, so there is no way to listen to this event.

justinjung04 commented 5 years ago

Thank you for making a PR! I agree that it will be ideal for outer apps to listen to webviewer events in a nice uniform way, but currently they are split into different objects. For example, annotationSelected event is triggered in annotation manager, meaning you can listen to the event as follows:

viewerElement.addEventListener('ready', () => {
  const viewerInstance = viewer.getInstance();
  const annotManager = viewerInstance.docViewer.getAnnotationManager();
  annotManager.on('annotationSelected', (e, annotList, action) => {
    // do something
  });
});

There are many other events that are not bubbled up to the outer app, and I think we need some sort of refactoring to make this easier for everyone. I really appreciate your PR, but I think a more fundamental fix on our end is needed. What do you think?

glendaviesnz commented 5 years ago

A more fundamental fix for event bubbling would be great - do you know when this is likely to make it onto your roadmap?

justinjung04 commented 5 years ago

This would require fair bit of change and refactoring, so we don't have a timeline yet. We are however putting more effort into improving APIs in general, which will happen incrementally and this change will be part of it one day.

glendaviesnz commented 5 years ago

ok - if you would prefer to leave this pull request out and wait for a more permanent fix to bubbling events that is fine, we will just apply the fix locally and wait for an improved events system - or if you are happy for it to be in the code in the mean time go ahead and merge it in - will leave it to you to decide.

justinjung04 commented 5 years ago

I will close this issue (and the pr) for now, and reference it in our backlog for improving event system. Thank you for the pr, and hopefully we come up with an improvement as soon as possible :)