Bug 15269 - Layout tweaks for the opac-detail page
Summary: Layout tweaks for the opac-detail page
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jen Zajac
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2015-11-30 23:22 UTC by Jen Zajac
Modified: 2019-07-23 15:06 UTC (History)
9 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15269: Layout tweaks for the opac-detail page (89.45 KB, patch)
2015-11-30 23:55 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269: Compiled CSS changes (83.33 KB, patch)
2015-11-30 23:55 UTC, Jen Zajac
Details | Diff | Splinter Review
Screenshot of updated layout (116.26 KB, image/png)
2015-11-30 23:58 UTC, Jen Zajac
Details
Bug 15269 [ENH]: Compiled CSS changes (83.35 KB, patch)
2015-12-01 00:25 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Layout tweaks for the opac-detail page (89.48 KB, patch)
2015-12-01 00:28 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Compiled CSS changes (83.35 KB, patch)
2015-12-01 00:28 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Layout tweaks for the opac-detail page (91.81 KB, patch)
2015-12-02 02:02 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Compiled CSS changes (83.35 KB, patch)
2015-12-02 02:02 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Layout tweaks for the opac-detail page (84.47 KB, patch)
2015-12-02 22:44 UTC, Jen Zajac
Details | Diff | Splinter Review
Bug 15269 [ENH]: Compiled CSS changes (83.35 KB, patch)
2015-12-02 22:44 UTC, Jen Zajac
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15269 [ENH]: Layout tweaks for the opac-detail page (84.53 KB, patch)
2015-12-07 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15269 [ENH]: Compiled CSS changes (83.42 KB, patch)
2015-12-07 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15269 [ENH]: Layout tweaks for the opac-detail page (84.64 KB, patch)
2016-01-19 14:41 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15269 [ENH]: Compiled CSS changes (83.51 KB, patch)
2016-01-19 14:42 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jen Zajac 2015-11-30 23:22:54 UTC
Make the following improvements to the layout of the OPAC detail page:

* Image thumbnail has more white space
* Summary is moved up the details block and font size is increased
* Ratings controls are moved up the details
* Small devices get a better layout
Comment 1 Jen Zajac 2015-11-30 23:55:23 UTC Comment hidden (obsolete)
Comment 2 Jen Zajac 2015-11-30 23:55:42 UTC Comment hidden (obsolete)
Comment 3 Jen Zajac 2015-11-30 23:58:11 UTC
Created attachment 45249 [details]
Screenshot of updated layout
Comment 4 Jen Zajac 2015-12-01 00:25:35 UTC Comment hidden (obsolete)
Comment 5 Jen Zajac 2015-12-01 00:28:16 UTC Comment hidden (obsolete)
Comment 6 Jen Zajac 2015-12-01 00:28:28 UTC Comment hidden (obsolete)
Comment 7 Jen Zajac 2015-12-01 00:30:40 UTC
Sorry, the changes were just for commit message formatting.
Comment 8 Owen Leonard 2015-12-01 14:18:31 UTC
My initial reaction is that this looks really great. I'm going to test, but I want to point out that this bug has no test plan where it really needs a very vigorous one. There are a lot of OPAC settings which need to be tested on and off, and with XSLT turned on and off.
Comment 9 Héctor Eduardo Castro Avalos 2015-12-01 14:58:30 UTC
(In reply to Owen Leonard from comment #8)
> My initial reaction is that this looks really great. I'm going to test, but
> I want to point out that this bug has no test plan where it really needs a
> very vigorous one. There are a lot of OPAC settings which need to be tested
> on and off, and with XSLT turned on and off.

You're right Owen.

Nice work Jen, the screen layout looks much better.
Comment 10 Héctor Eduardo Castro Avalos 2015-12-01 17:25:35 UTC
Hi Jen

Average rating disappear when there not exists summary in record. Test with another book without summary (520)
Comment 11 Jen Zajac 2015-12-01 22:35:36 UTC
Good catch, Héctor. I had tried to make it agnostic to the summary being there but it's a bit tricky. I will see if I can sort that out.

Owen, regarding the test plan, my apologies it doesn't seem to be in the developer workflow on the wiki (this is my first proper Koha patch). Any chance you could point me at something on the wiki, or another bug I could look at as an example to see what that would look like? And then I can sort that out and get it added. Is it just like the "to test bit" in the commit message on the commit message page on the wiki? Like you say, I think a formal test plan for this would be a lot more complicated than that example, would the commit message be the right place for it?
Comment 12 Jen Zajac 2015-12-02 02:02:38 UTC Comment hidden (obsolete)
Comment 13 Jen Zajac 2015-12-02 02:02:47 UTC Comment hidden (obsolete)
Comment 14 Jen Zajac 2015-12-02 02:07:01 UTC
Hi, I've updated the patch to make the logic for moving the ratings controls upwards a lot more robust and agnostic of the presence of particular fields. I've also added a crude test plan. I'm _not_ a Koha expert and it might be missing some necessary detail about specific configurations of fields that I'm unaware of, but hopefully it's a good starting place.
Comment 15 Héctor Eduardo Castro Avalos 2015-12-02 06:20:17 UTC
Hi Jen

I think (IMHO) if you can divide in more divs the details, i.e., all data is contained in <div id="detail" class="span10"> and it could look much better if you separate in two or three divs depends on the data displayed. If record is too long -like motion pictures, dvds, etc- a white block is displayed below the cover image, so this way the second div can be aligned at left and so on. Here we must consider that two or more covers can be deployed. Also Koha notice this in Enhanced Content preferences in All section "NOTE: you can only choose one source of cover images from below, otherwise Koha will show the images from all sources selected."

If OPACLocalCoverImages is set to 'Display' and there is not a cover image <div class="span2"> must be hide or display something like "No cover image available" in a box.

I can not have access to Enhanced Content that require account and password like NoveList Select, If you have access, please paste a link for video like Kyle Hall did it in bug <http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15079>. I don't know if QA Team have access to this.

I've been tested this with syprefs GoogleJackets, OpenLibraryCovers, OPACAmazonCoverImages and works excellent, I will test it with more syprefs involved.

Could be great if you extend this to UNIMARC and NORMARC environment.

Regards
Comment 16 Jen Zajac 2015-12-02 22:44:22 UTC Comment hidden (obsolete)
Comment 17 Jen Zajac 2015-12-02 22:44:34 UTC Comment hidden (obsolete)
Comment 18 Jen Zajac 2015-12-02 22:45:29 UTC
Hi Héctor,

Thanks again for you work looking into this. It looks like the local cover images not having a proper placeholder is a bug as a 1x1 pixel image is returned. There is a bug that addresses this at: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11602

I'm aware that there is whitespace under the cover image (or images if you have more than one source turned on). This is by design, IMO it makes the page look less cluttered. Having a narrower column for the main details is not necessarily a bad thing, there are studies that say that slightly narrower columns are easier to read (see e.g. http://graphicdesign.stackexchange.com/questions/13724/recommended-column-width-for-text-reading-digital-vs-printed).

I had a look at porting some of the changes to NOMARC and UNIMARC but I'm a bit leery of messing with them as I'm not sure I'm grokking some of the data structures. I could help with porting the changes from this patch to those systems as a later patchset. In the meantime, those systems will still get some but not all of the changes from this patch: the cover image column, larger font size for the authors, ratings controls moved up under the authors. They won't get the item summary moved up and the larger font sizes for series, summary and item type.

I have made one change in the most recent update. The 773 data in MARC21slim2OPACDetail.xsl is moved back down to its original location. I had moved it up along with the summary field, but I don't think any more that they need to move together, and leaving it in its original position reduces the scope of this change.

I don't unfortunately have any credentials either, but I am checking with my colleagues if we can shoulder tap someone who does to do a bit more testing on that front. 

Thanks,

Jen
Comment 19 Héctor Eduardo Castro Avalos 2015-12-03 03:04:32 UTC
You're right Jen if you apply 11602 on top the placeholder is fixed it up.

For me status need to change to Needs sign-off
Comment 20 Nick Clemens 2015-12-07 16:08:00 UTC Comment hidden (obsolete)
Comment 21 Nick Clemens 2015-12-07 16:08:09 UTC Comment hidden (obsolete)
Comment 22 Nick Clemens 2015-12-07 16:09:04 UTC
Tested with all non-subscription cover sources and novelist, all looks really good!
Comment 23 Jonathan Druart 2015-12-08 15:19:47 UTC
Owen, could you have a look at this one please?
Comment 24 Jonathan Druart 2016-01-19 09:12:31 UTC
I really would like to get another signoff on this one.
Comment 25 Owen Leonard 2016-01-19 14:41:00 UTC
Created attachment 46885 [details] [review]
[SIGNED-OFF] Bug 15269 [ENH]: Layout tweaks for the opac-detail page

Make the following improvements to the layout of
the OPAC detail page:

* Image thumbnail has more white space
* Summary is moved up the details block and font
  size is increased
* Ratings controls are moved up the details
* Small devices get a better layout

To test:
- Check that the ratings controls are moved up the
  page to be before the summary
    * If there is no summary, the controls should
      be under the series
    * If there is no summary or no series, the
      controls should be under the type
    * If there is no summary, no series, or no type,
      the controls should be under the authors
      (including contributing authors)
- Check that the 520 field (Summary, Subject, Review,
  Scope and content, Abstract, Content advice) is
  rendered nearer to the top of the page, following
  material type, series, and the ratings.
- Check that the font size for "Material type",
  "Series" and the 520 field (Summary, Subject,
  Review, Scope and content, Abstract, Content advice)
  are rendered at a 14px font size.
- Check that authors and contributing authors are
  rendered at 16px font size.
- Check that if any kind of cover image is present
  (or the no cover image placeholder image) it is
  rendered into a div with a class of 'span2' and
  appears on the left of the main item details.
    * Need to test every kind of cover image service
    * If no kind of cover image service is turned on,
      then the 'span2' div should not be present and
      the item details should be on the left of the
      content area.
- Check that small devices (e.g. mobile phones) make
  the 'span2' image column take up the full width and
  appear above the rest of the item details.
- Check that layout of the search results page is
  unaffected.
- Check that changing 'OPACXSLTDetailsDisplay' to an
  empty string still yields the expected layout.

Sponsored-by: ByWater

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 26 Owen Leonard 2016-01-19 14:42:33 UTC
Created attachment 46886 [details] [review]
[SIGNED-OFF] Bug 15269 [ENH]: Compiled CSS changes

Sponsored-by: ByWater

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 27 Jonathan Druart 2016-01-19 16:14:44 UTC
If JS is disabled, the .span2 is empty but takes a lot of space (even with small devices).
If JS is enabled and you don't have a local cover image (and pref OPACLocalCoverImages is set), the space takes by the span2 is much more wide than the "no cover" image.

I'd say it's not blocker, but I'd prefer someone confirm it's the expected behavior.
Comment 28 Liz Rea 2016-02-16 01:54:59 UTC
Hi Jonathan,

Yep, that should probably be fixed we'll take a look into a follow up for that.

Cheers,
Liz
Comment 29 Francesca Moore 2016-02-24 23:52:52 UTC
When trying to apply patch I am present with the following error:
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
Comment 30 Gracie 2017-01-18 22:03:06 UTC
I tried to resolve the conflict that occurs while applying this patch, the conflict is very big and confusing