readium / readium-shared-js

Repository for the shared JavaScript libraries that are used in the SDK-Launchers and other applications developed on top of the SDK
BSD 3-Clause "New" or "Revised" License
78 stars 102 forks source link

Images not resizing to fit the screen in scrollview #64

Open danielweck opened 10 years ago

danielweck commented 10 years ago

Originally posted by @vanisuthamathi https://github.com/readium/readium-js/issues/57

Steps to Reproduce:

  1. Enable scroll view - readium.reader.updateSettings({ scroll: "scroll-doc" }); in the Epub reader view
  2. Read any article containing big image sizes like 600x511, 600X300 and etc., Observation:

    Images are not resizing to fit the screen as shown in the attached image. image_notresizing

Note: We have fixed in our application by adding css properties to the image tags after the content documents are loaded $(imgTags).css('max-width', '98%'); $(imgTags).css('max-height', '98%');

Please fix in the readium library

danielweck commented 10 years ago

See candidate fix in feature branch: https://github.com/readium/readium-shared-js/tree/feature/scroll_img_resize

UPDATE: feature branch now deleted, but commit preserved as reference (proposal: scroll view acts like regular scrolling webpage => no arbitrary resize-to-fit imposed by reading system, just application of authored CSS styles).

Code diff: https://github.com/readium/readium-shared-js/commit/30c0ecda5dc995bd154f9f7bb49ac9991c6c7c9a

I do not think that this is a fully-satisfactory solution though, as many books I tried have image width/height either set via CSS or attributes, and the application of maximum values irrespective of aspect ratio sometimes produces unexpected / undesirable results. Furthermore, some images do not resize when opening the page for the first time, only when resizing the viewport.

In fairness though, the same problem already exists with the columnized paginated view: https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L669

...so one could argue that the behaviour should be consistent across all types of reflowable views, and that we should apply the same image resizing algorithm to the scroll view?

danielweck commented 10 years ago

See also: #45

danielweck commented 10 years ago

See also: https://github.com/readium/readium-shared-js/issues/88

danielweck commented 10 years ago

Shared utility function between scroll and column-paginated views:

https://github.com/readium/readium-shared-js/commit/a64dc4d7192ed45271104ce586cc01483f969744

Feature branch:

https://github.com/readium/readium-shared-js/tree/feature/scroll_img_resize

UPDATE: feature branch now deleted, but commit preserved as reference (proposal: scroll view acts like regular scrolling webpage => no arbitrary resize-to-fit imposed by reading system, just application of authored CSS styles).

rkwright commented 10 years ago

I created a test file and verified this bug in the current build of the Chrome Extension. The file is here: https://readiumfoundation.box.com/s/c9rxcdaszp39dswiw7su

ryanackley commented 10 years ago

We'll need to add this for svg's as well in your feature branch. It's just a matter of adding 'svg' to the jquery selector we use.