PDF - Generalise front cover and back cover image

patkyn commented 7 years ago

Front and back cover banner image is currently specific for Flora of Australia collection. This needs to be changed for other collections or have collection configuration setting.

It would also be good if footer colour was configurable.

RobinaSanderson commented 7 years ago

Hi @sbearcsiro, Michael asked me to test the stories in code review. I've just looked at the .pdfs generated from https://profiles-dev.ala.org.au/opus/alatest and the cover pafe still reflects the Flora of Australia.

Am i looking in the wrong place? :-)

sbearcsiro commented 7 years ago

I suspect that these changes haven't been deployed to dev yet...

RobinaSanderson commented 7 years ago

Checked Front and back cover.

Front cover banner reflects the collection name for the collection rather than Flora of Australia and uses the text colour specified in the collections configurable theme settings.

Back cover appears to use the call to action colour from the configuration settings for the colour of the back page.

Footer colour uses the colour set in the collection theme.

Test passed

m-hope commented 7 years ago

Some minor comments: References to the Flora of Australia have been removed and colours use the collections defined theme colours, however the images used at the top of the front and back pages appear to still be hardwired... need to consider the options to change/set these images as they may not be appropriate for other collections (particularly if they are not flora based).

Also the ALA logo on the back page didn't turn out well for the PDF generated from https://profiles-dev.ala.org.au/opus/mlh/profile/Batis%20argillicola.

Need to chase: generating a PDF for https://profiles-dev.ala.org.au/opus/alatest/profile/Acacia%20dealbata, produces an error...may have something to do with lower taxa...

sbearcsiro commented 7 years ago

@m-hope cover images should already be configurable via Opus Configuration -> Branding -> Banner image for export cover / Banner image for export back cover. For the mlh opus they don't appear to be set: https://profiles-dev.ala.org.au/opus/cbc7e26c-fb2d-4c5e-96af-fbc2882cb57f/update

Agreed on the back cover ALA logo. Currently that logo image has a transparent background and the box that contains it uses the footer background colour for its background colour. My feeling is that this box should remain hard coded to its previous value given the logo image isn't configurable. Or we can take an existing colour and adjust the saturation and lightness levels to get a colour that will contrast with the logo nicely but still fits the users colour scheme.

This is the exception from alalatest Acacia dealbata PDF generation:

groovy.lang.GroovyRuntimeException: Ambiguous method overloading for method java.util.LinkedHashMap#putAll.
Cannot resolve which method to invoke for [null] due to overlapping prototypes between:
        [interface java.util.Map]
        [interface java.util.Collection]
        at au.org.ala.profile.hub.ExportService$_groupImagesIntoPairs_closure27.doCall(ExportService.groovy:765)
        at au.org.ala.profile.hub.ExportService.groupImagesIntoPairs(ExportService.groovy:754)
        at au.org.ala.profile.hub.ExportService.loadProfileData(ExportService.groovy:491)
        at au.org.ala.profile.hub.ExportService.getCurateReportModel(ExportService.groovy:311)
        at au.org.ala.profile.hub.ExportService.createPdf(ExportService.groovy:114)
        at au.org.ala.profile.hub.ExportController.getPdf(ExportController.groovy:35)
        at grails.plugin.cache.web.filter.PageFragmentCachingFilter.doFilter(PageFragmentCachingFilter.java:198)
        at grails.plugin.cache.web.filter.AbstractFilter.doFilter(AbstractFilter.java:63)
        at au.org.ala.cas.client.AlaHttpServletRequestWrapperFilter.doFilter(AlaHttpServletRequestWrapperFilter.java:81)
        at au.org.ala.cas.client.UriFilter.doFilter(UriFilter.java:196)
        at au.org.ala.web.filter.ParametersFilterProxy.doFilter(ParametersFilterProxy.java:24)
        at org.jasig.cas.client.validation.AbstractTicketValidationFilter.doFilter(AbstractTicketValidationFilter.java:236)
        at au.org.ala.cas.client.UriFilter.doFilter(UriFilter.java:196)
        at au.org.ala.web.filter.ParametersFilterProxy.doFilter(ParametersFilterProxy.java:24)
        at org.jasig.cas.client.authentication.AuthenticationFilter.doFilter(AuthenticationFilter.java:155)
        at au.org.ala.cas.client.UriFilter.doFilter(UriFilter.java:196)
        at au.org.ala.web.filter.ParametersFilterProxy.doFilter(ParametersFilterProxy.java:24)
        at org.jasig.cas.client.session.SingleSignOutFilter.doFilter(SingleSignOutFilter.java:97)
        at com.brandseye.cors.CorsFilter.doFilter(CorsFilter.java:82)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:748)

Which looks like a general PDF generation bug, but should be a simple fix.

RobinaSanderson commented 7 years ago

Hi @sbearcsiro @m-hope - My preference would be for a hardcoded background to our logo plus any others. I think we don't really want to be messing with logo colours, either letting people mess with ours or allowing them to mess with other organisations. I can have a chat with Larissa to confirm if you like? If we do decide to hardcode the background i think we should pull that into a specific story.

Cheers Robina

m-hope commented 7 years ago

Sorry @sbearcsiro, you are correct with the PDF banner images, I did the test late yesterday and didn't check if there was already an option... tested that now and it works fine... I do think though that we need to change the default image to something neutral and force the eFlora of Australia collection to specify their image in the config... this can be a separate story to be done later (@RobinaSanderson, I'll start putting these issues at the top of To Do).

Also agree that the ALA logo on the back page should be hardcoded and non-transparent, but would prefer this to be fixed now if possible. Simon, how easy would this be to do if we can source the correct image?

I'll have a play with the PDF generation error... it may just be bad data, as I can't recreate it with anything other than the A. dealbata profile in ALATest so far...

sbearcsiro commented 7 years ago

@m-hope Cool - we can just set the colour of the box the contains the logo, should take about 5 minutes to find the change that applies the styling to that box in the previous commits and revert it.

m-hope commented 7 years ago

Confirm this issue can be closed once code review done.
The PDF error on generation is a separate issue to do with private images (#589). Also the change to fix the ALA logo on the back page has a separate issue (#588)

patkyn commented 6 years ago

https://upsource.ala.org.au/profiles/review/PRFLS-CR-193

elywallis commented 6 years ago

I have re-tested the specific issue of selecting front and back cover images. Using https://aws-profiles.ala.org.au/opus/ala-design/

I opened Edit Configuration, Branding
I uploaded a file at the recommended pixel dimensions into Banner image for export cover
I uploaded a file at the recommended pixel dimensions into Banner image for export back cover NOTE that it took me a few goes to work out how to save these settings as you have to scroll quite a way to get to the Save button (under License - long description)
I returned to Public View and browsed to a species profile (Berty cunninghamii)
I exported a pdf and confirmed that the front and back cover images were the ones I'd just uploaded.

AtlasOfLivingAustralia / profile-hub

PDF - Generalise front cover and back cover image #563