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

Added ImageViewerPlugin, edited PluginLoader for images extensions. #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Strae
Copy link

@Strae Strae commented Feb 11, 2014

Clean add of ImageViewerPlugin.

@kogmbh-ci
Copy link

Can one of the admins verify this patch?

@thz
Copy link
Member

thz commented Feb 11, 2014

ok to test

@kogmbh-ci
Copy link

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/ViewerJS-PullReq/24/

@thz
Copy link
Member

thz commented Feb 11, 2014

hmmm it fails with a 404 on ImageViewerPlugin.js
see http://ci.kogmbh.com/deploy/ViewerJS/ViewerJS-ci-pr28-b24-g3509c686/index.html#/deploy/book.png

@thz
Copy link
Member

thz commented Feb 11, 2014

could it be that you forgot your cmakelists.txt changes?

@Strae
Copy link
Author

Strae commented Feb 11, 2014

Indeed, I didnt edit the that file, should I only add a row like:

COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/ImageViewerPlugin.js ${VIEWER_BUILD_DIR}/ImageViewerPlugin.js

?

@thz
Copy link
Member

thz commented Feb 11, 2014

indeed.
please add that line to cmakelists.txt in your branch and commit it - this should also update this PR implicitly.

@Strae
Copy link
Author

Strae commented Feb 11, 2014

Ok done, sorry for trouble ;)

@kogmbh-ci
Copy link

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/ViewerJS-PullReq/25/

@thz
Copy link
Member

thz commented Feb 11, 2014

hmm: http://ci.kogmbh.com/deploy/ViewerJS/ViewerJS-ci-pr28-b25-g7cd2c483/#/deploy/book.png

now there is an issue with:
Uncaught TypeError: Object # has no method 'getPluginName'

did that actually work for you?

@kogmbh-ci
Copy link

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/ViewerJS-PullReq/26/

@Strae
Copy link
Author

Strae commented Feb 11, 2014

Mhh yes, you can see the first try here: http://meanbit.com/node/1 (dont mind the red errors on the top, that is a Drupal error I already fixed in the plugin.. and it is pretty slow loading, it is my test public server)

Looks like for the usage I did that function never got invoked, by the way i'll add now..

Is possible to have a list (documentation) of all the functions that must be implemented?
I took pieces from ODF and PDF plugins, but probably I missed those which didnt give me any error..

@thz
Copy link
Member

thz commented Feb 11, 2014

we currently have no such set of documentation. we would welcome such a contribution of course :-)

please let us know if you need help adding the missing functions. and you can always use the deployment links which are posted here in github.

@adityab
Copy link
Member

adityab commented Mar 12, 2014

At this point it errors out with TypeError: c.getPluginVersion is not a function and I cannot open an image.

@Strae
Copy link
Author

Strae commented Mar 13, 2014

Sorry guys im out of town until monday, then i'll fix all those missing functions in the image plugin

@kogmbh-ci
Copy link

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/ViewerJS-PullReq/28/

@Strae
Copy link
Author

Strae commented Mar 17, 2014

Done, please note I jsut copyed and pasted the function that I thought could be invoked from the outside.. ahhw, when I get some time i'll try to dive in the code to pull out some documentation about those functions..

@thz
Copy link
Member

thz commented Apr 3, 2014

http://ci.kogmbh.com/deploy/ViewerJS/ViewerJS-ci-pr28-b28-g645689f5/index.html#/deploy/book.png

this seems to work for me now.

default fit-to-width is debatable, but perhaps not the worst decision.

@adityab any more comments/remarks?

@kossebau
Copy link
Contributor

kossebau commented Apr 3, 2014

Choosing "Automatic" in the zoom menu does not have any effect for me at all, independent on the current zooming state. On purpose? What should happen?

Also does fit-to-width go beyond the max. zoom level reachable by clicking the "+" button, which is "400 %". And the zoom menu only offers "200 %" as max. value, seems inconsistent.

@kossebau
Copy link
Contributor

kossebau commented Apr 3, 2014

Going to presentation mode results with above example results in scrollbars. I expect no scrollbars in presentation mode, but rather fit-to-view. What should happen in presentation mode?

@kossebau
Copy link
Contributor

kossebau commented Apr 3, 2014

The page selector in the lower toolbar also needs work. It shows "0" for the number of pages, and starts with "1" for the currently displayed "page". Clicking the page up/down buttons (which are not disabled) results in switching between "0" and "1" for the current page, without any effect on what is displayed.
Perhaps viewerjs shell should prepare to also display non-pages objects, and in that case remove the page selector.

@kossebau
Copy link
Contributor

kossebau commented Apr 3, 2014

The link from "ImageViewerPlugin.js" in the about dialog goes to https://github.com/mozilla/pdf.js/. Instead it should go to where?

@thz
Copy link
Member

thz commented Apr 3, 2014

the plugin is at home in ViewerJS itself, so there is no external upstream to refer to.

@Strae
Copy link
Author

Strae commented Apr 4, 2014

Yes there are some issues with the zoom, i'll give it a check

@kossebau kossebau added this to the ViewerJS some future version milestone Jul 14, 2014
@lfilho
Copy link

lfilho commented Aug 29, 2014

+1 for this.

@lfilho
Copy link

lfilho commented Aug 29, 2014

Does this plugin also supports TIFF images?

@Strae
Copy link
Author

Strae commented Aug 29, 2014

This plugin actually does nothing except to show the image via an simple
HTML IMG tag..
So if the browser support the image format, the plugin will work fine..
Would be great to add support for other formats!
Il 29/ago/2014 19:39 "Luiz Gonzaga dos Santos Filho" <
[email protected]> ha scritto:

Does this plugin also supports TIFF images?


Reply to this email directly or view it on GitHub
#28 (comment).

@kossebau kossebau mentioned this pull request Feb 19, 2015
h44z pushed a commit to h44z/ViewerJS that referenced this pull request Jun 16, 2015
@evivz
Copy link

evivz commented Mar 28, 2016

Any documentations on how to use this feature?
Basically I want to load multiple images instead of pdf.

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

Successfully merging this pull request may close these issues.

7 participants