Skip to content
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

API for customizing thumbnails in thumbnail panel #49

Closed
sabieber opened this issue Nov 16, 2018 · 22 comments
Closed

API for customizing thumbnails in thumbnail panel #49

sabieber opened this issue Nov 16, 2018 · 22 comments
Labels
enhancement New feature or request

Comments

@sabieber
Copy link
Contributor

As already discussed in the google group (topic) we need an API for customizing the rendered content of the Thumbnails in the ThumbnailsPanel.

In my opinion it would be best for everyone if this was a generic API that could be used by a lot of users so I would like to implement such API.

Would there be requirements that I need to match so the proposed API would be merged by you?

My first idea would be a setter for providing an activator/customizor function that is called when a Thumbnail is rendered to alter the DOM contents. Does this sound like a good idea for general use?

What we really need is to display two small buttons next to the page number on the Thumbnail (one for download and one for another feature of our customized viewer).

sabieber added a commit to sabieber/webviewer-ui that referenced this issue Nov 20, 2018
@justinjung04
Copy link
Contributor

I've reviewed your PR, and it looks like it's an API to append custom elements after the page number div.
We would like to keep this project general enough, and the PR seems a bit specific to number of use cases only.
To come up with a more general solution, do you think it would be useful for an API to get thumbnail images, which could be used to create custom thumbnails from scratch?

@sabieber
Copy link
Contributor Author

Thanks for reviewing my PR and providing feedback.

You are right in the assumption that its an API for appending custom DOM elements to the end of the Thumbnail (so its below the page number - in our case we would hide the normal page number via CSS).

I actually think the provided API could be of use in a lot of different cases, for example one culd use it to overlay the thumbnail with custom content by using the API and some basic CSS rules.
Also we would actually like to introduce a similar API to annotations in the annotations panel to introduce some custom elements in the list view).

If you think the API is not general enough we could work with the API you suggested if it is implemented. I don't really know if there is a way to build something similar for the annotations though, as they have more complex elements like dropdowns, replies, ...

Would be perfect if we could implement these APIs rather quickly as we want to roll out the new UI in december.

@justinjung04
Copy link
Contributor

I also agree that there could be many users who want to tweak existing thumbnails panel and notes panel, but I just don't think having an API to simply append custom element is sufficient. It's true that the API could be powerful if used with CSS, but that also means the customization code becomes more complex and hard to understand for people who are not familiar with CSS.

@sabieber
Copy link
Contributor Author

Is this something you would design and implement then? From our perspective its not really feasable to make the implemented API more general to support more usecases.

How would you suggest a similar API would look like for the customization of the annotation card in the annotations panel (to support adding a status and buttons)?

@justinjung04 justinjung04 added the enhancement New feature or request label Dec 3, 2018
@justinjung04
Copy link
Contributor

This is one of the enhancement we have on our backlog, but we don't have a timeline yet.

API for customizing annotation card could be trickier, as it involves more components and actions. What kind of customizations are you thinking of?

@sabieber
Copy link
Contributor Author

The customizations for the annotations we currently need are adding custom actions to the dropdown (where edit and delete are) and showing a custom status inside the card (something like unrevised/finished).

We currently have a branch in my own fork of the ui with an API which allows us to add a custom div to every annotation card in the sidebar. You can check it out here.
I think it would be really good to have something similar (more generic is fine) for every user of the webviewer.

You can check out our current version of the APIs here:
sabieber@29f340b
sabieber@9199964

Hope we can further discuss this and find a solution that is suitable for adding as a general API

@sabieber
Copy link
Contributor Author

Any news on this?

@justinjung04
Copy link
Contributor

justinjung04 commented Jan 14, 2019

Sorry for a late reply, and thank you for clarifying the use case and sharing the code.
We had an internal discussion, and thought of an API that might look like this:

viewerInstance.setThumnailRender(({ pageNumber, thumbnail }) => {
  // pageNumber is a string
  // thumbnail is a DOM element
  const result = document.createElement('div');
  const customLabel = document.createElement('div');
  customLabel.innerHTML = 'This is page ' + pageNumber;
  result.appendChild(customLabel);
  result.appendChild(thumbnail);
  return result;
});

which will render the custom page label on top of thumbnail image.
Any comments or feedback would be appreciated.

@sabieber
Copy link
Contributor Author

Thanks for discussing the issue internally and coming up with a proposal.
The proposed API looks fine and should solve our use case as well as being flexible enough for other users.

When do you think this could be implemented?
Were there any discussions yet regarding the customized annotations content?

@sabieber
Copy link
Contributor Author

@justinjung04 are there some news on this? Thanks

@justinjung04
Copy link
Contributor

Sorry for a late reply.
We have been busy preparing for a next release over the last month.
I will make sure to work on it and give you an update after we release WebViewer 5.0 :)

@ZhijieZhang
Copy link
Contributor

Hi Sascha,

You can find the API available in thumbnail branch: https://github.com/PDFTron/webviewer-ui/tree/thumbnail

Here is how you can use the API to render the page number and a download "button" above the thumbnail:

viewerInstance.setThumbnailRenderer(({ pageNumber, thumbnailContainer }) => {
  const result = document.createElement('div');

  const pageLabel = document.createElement('div');
  pageLabel.innerHTML = `${pageNumber}`;

  const downloadButton = document.createElement('div');
  downloadButton.innerHTML = 'download';
  downloadButton.addEventListener('click', () => { viewerInstance.downloadPdf() })

  result.appendChild(pageLabel);
  result.appendChild(downloadButton);
  result.appendChild(thumbnailContainer);

  return result;
})

Let us know how this API works for you. If everything is good then we will merge it to master.

Thanks!

@sabieber
Copy link
Contributor Author

@ZhijieZhang thanks for the branch and example. This should work for us pretty well! Thanks for the work!

@sabieber
Copy link
Contributor Author

I ported our thumbnail customization to the new proposed API for testing and it works like expected. So this should be merged into master, thanks 👍

@sabieber
Copy link
Contributor Author

sabieber commented Mar 20, 2019

Actually we currently get an exception when setting the thumbnail renderer in viewerLoaded of our config resulting in the first two thumbnails not being rendered:

Uncaught TypeError: Cannot read property 'appendChild' of null
    at webviewer-ui.min.js:47
    at webviewer-ui.min.js:38
    at Image.e.onload (CoreControls.js:784)

Is there a fix for this?

@ZhijieZhang
Copy link
Contributor

ZhijieZhang commented Mar 20, 2019

I tried to run the code snippet posted above in a config file but didn't see any errors...
Could you elaborate more about how you are using this API? Being able to see your related code in your config file would be very helpful.

@sabieber
Copy link
Contributor Author

Thanks for looking into this issue :)

The issue can be reproduced in our testing system under the following URL https://oxomi.test.scireum.com/p/DEMO/catalog/10001937
Our configuration can be found as config.js in the sources panel of the browser debugger and the function that calls setThumbnailRenderer is called Config.overrideThumbnails. It seems to be some sort of timing issue as it works with a config just containing the setThumbnailRenderer call.
The error occurs since switching to your new API, before with our proposed setThumbnailCustomContentRenderer everything worked fine.

@ZhijieZhang
Copy link
Contributor

ZhijieZhang commented Mar 22, 2019

Hey thanks for sending us the URL. We were able to reproduce the issue on your testing system.

However if we put a breakpoint on where the exception throws, thumbnail will be loaded correctly as we step over and we are not be able to see the error anymore.

We found one suspicious spot in Thumbnail.js which may cause this error and pushed a fix for that. Can you try building from thumbnail branch and see if the issue is resolved? If the issue still occurs, can you try calling Config.overrideThumbnails in documentLoaded event?

@sabieber
Copy link
Contributor Author

Thanks for looking into the issue, but the thumbnail branch was not updated

@sabieber
Copy link
Contributor Author

We tried to merge the current master in my thumbnail branch and build from there but the issue still occurs.
We also tried to call Config.overrideThumbnails in documentLoaded instead but this doesnt seem to work either.

Sadly if the issue can't be resolved I would have to switch back to our own implementation proposal for our next release

@ZhijieZhang
Copy link
Contributor

Oops, forgot to push the changes to origin thumbnail. Sorry!
Could you please try again with thumbnail branch and see if the issue still occurs.

@ZhijieZhang
Copy link
Contributor

I'm closing this issue due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants