Sync body height with CKAN data preview min. height#26
Conversation
This prevents large amounts of black space at the bottom of the PDF
|
Would appreciate a review and/or merge on this |
torfsen
left a comment
torfsen
left a comment
There was a problem hiding this comment.
I can reproduce the original issue (empty black space at the end of the PDF display) and this PR fixes it. However, I've only tested this on CKAN 2.7. @mattfullerton, what about other versions?
Regarding the removal of resource_view.html: The file was originally added in 8c73c14 to add support for presentation mode -- that doesn't work for me anymore with this PR, the corresponding button in the toolbar of the PDF viewer is simply missing. So that's something that needs to be fixed.
mattfullerton
force-pushed
the
master
branch
2 times, most recently
from
1bc5310 to
a7306e3
Compare
including latest changes from CKAN core and including helpers to cope with the many possible frontend configurations for CKAN templates across different versions
|
@torfsen I've restored resource_view.html, incorporating the changes from the latest CKAN version. This brought up the issue of bootstrap and the different ways of referencing icons from templates, so I added some helper functions to cope with that. I've tested this with 2.4 and 2.7 and will test master (2.8/bootstrap 3). I don't have time to deal with 2.5/2.6 (I used an old VM I had for 2.4, docker has been stressing me out lately). |
|
@torfsen Also working on master |
|
@torfsen I have tested this extensively (see above) and also added a cosmetic commit to trigger Travis tests, which are successful (thanks for fixing Travis @torfsen - I merged your commit in this repo and also pasted in the change from ckanext-page!), except for 2.3 where the build died because of a communications error when downloading packages. Possibly we want to drop support for 2.3 anyway? @amercader Possible to merge? |
|
@torfsen @mattfullerton you should have access to the repo. Please feel free to merge any PRs needed as you are much more up to date with this extension than me (and probably than anyone else). Of course ping me if you have any doubt, but I have full confidence in you :) 2.3 can be dropped if it causes any problem at all. |
|
I've merged #28 now and merged master into this fork, and for some reason get a couple of test errors. I'm not familiar with this code, so maybe @mattfullerton you might fix these last remaining things and I can merge this? |
5 participants
This prevents large amounts of black space at the bottom of the PDF