Bug 8884 - opacheader is not in the header on CCSR
Summary: opacheader is not in the header on CCSR
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Owen Leonard
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-05 19:45 UTC by Nicole C. Engard
Modified: 2014-12-07 20:02 UTC (History)
8 users (show)

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


Attachments
opacheader to the right of search (16.31 KB, image/png)
2012-10-05 19:45 UTC, Nicole C. Engard
Details
Bug 8884: opacheader is not in the header on CCSR (1.57 KB, patch)
2013-05-20 17:07 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8884: opacheader is not in the header on CCSR (1.62 KB, patch)
2013-05-24 21:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8884: opacheader is not in the header on CCSR (1.92 KB, patch)
2013-06-14 16:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10245: add CSS classes to OPAC detail items table header (2.91 KB, patch)
2013-06-14 23:07 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 8884: opacheader is not in the header on CCSR (1.97 KB, patch)
2013-06-14 23:07 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 8884: (follow-up) rearrange placement of the top-bar div in CCSR (10.92 KB, patch)
2013-06-14 23:08 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 8884: (follow-up) rearrange placement of the top-bar div in CCSR (10.98 KB, patch)
2013-06-23 07:10 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2012-10-05 19:45:20 UTC
Created attachment 12720 [details]
opacheader to the right of search

In CCSR the opacheader preference isn't really in the header it's to the right of the search box (see attached).  I'm okay with a customizable region over there, but we should have a new preference for that and header should be above the search box as it implies
Comment 1 Nicole C. Engard 2012-12-19 20:06:14 UTC
Changing assignee to Frédérick since he originally designed the theme and can probably fix this.
Comment 2 Maxime Pelletier 2012-12-23 17:17:02 UTC
(In reply to comment #1)
> Changing assignee to Frédérick since he originally designed the theme and
> can probably fix this.

Could you please remove Fréférick from all the ccsr tickets. He didn't design the theme (he did the mobile part and someone else from libeo and me did the rest) and he is not working anymore on Koha, so he won't be able to fix those bugs.

I will do my best to get some time to look at those bugs, but they are all very easy to fix, so no need to assign them.

Thanks.
Comment 3 Nicole C. Engard 2012-12-26 13:40:16 UTC
Assignee changed per comment on bug 8884.
Comment 4 Maxime Pelletier 2013-03-13 17:58:03 UTC
Changing assignee back to default because i'm not able to fix any ccsr theme bug as we don't use it yet.
Comment 5 Jared Camins-Esakov 2013-05-20 17:07:23 UTC Comment hidden (obsolete)
Comment 6 Ed Veal 2013-05-20 17:43:08 UTC
Jared,

I am going to test this but would like to make a suggestion. There are a few sites I am already working with and will be using the opacheader to place a library logo to the right of the search bar as it is functioning now. So if this passes QA and gets patched it will affect these sites. Might we be able to add a new system preference to work like the opacheader is working now on CCSR to accomodate this desired look? Just a thought. 

Ed
Comment 7 Owen Leonard 2013-05-20 18:49:09 UTC
(In reply to comment #6)
> There are a few
> sites I am already working with and will be using the opacheader to place a
> library logo to the right of the search bar as it is functioning now.

Is that because that's where they really want it, or because that's where they can put it? I would certainly prefer to fix the placement of opacheader and have any library customizations be corrected later. There will be plenty of notice before any such change arrives on customers' installations.
Comment 8 Ed Veal 2013-05-20 18:57:38 UTC
Owen,

Yes, I agree and that is what I was trying to say. I would like to opacheader work as it should and then add a new preference or just figure out a way to move the logos there. 

I was just bouncing around the idea. 

Ed
Comment 9 Katrin Fischer 2013-05-20 22:26:20 UTC
Maybe we could just copy from opacheader into the new preference on database update and preserve the current customizations that way?
Comment 10 Galen Charlton 2013-05-24 16:47:59 UTC
(In reply to comment #9)
> Maybe we could just copy from opacheader into the new preference on database
> update and preserve the current customizations that way?

Wouldn't copying over the contents of 'opacheader' to a new preference cause a bit of a surprise (in the form of duplicate text or the like) for libraries that aren't using CCSR?

Also, that begs the question of whether a new system preference is needed -- shifting logos around could be done via OpacUserCSS, and at the moment I'm not convinced that we need to maintain backwards compatibility for what appears to simply have been an oversight.
Comment 11 Chris Cormack 2013-05-24 21:20:05 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2013-06-14 16:10:09 UTC Comment hidden (obsolete)
Comment 13 Galen Charlton 2013-06-14 22:53:16 UTC
(In reply to comment #12)
> works as advertised with one caveat; the content of
> opacheader is by default underneath the div "top-bar". A couple breaks
> in my OpacHeader test made it visible, so I'm going to assume this is a
> styling issue and not a bug as such.

Indeed, a little CSS to add margin-top or padding-top to opac-custom-header would take care of moving it, but that would be a little unfriendly to a user who just wants to add a custom header without having to immediately wonder why the top bar overlaps it.  I'll shortly be attaching a follow-up patch for consideration.
Comment 14 Galen Charlton 2013-06-14 23:07:22 UTC Comment hidden (obsolete)
Comment 15 Galen Charlton 2013-06-14 23:07:48 UTC
Created attachment 19028 [details] [review]
Bug 8884: opacheader is not in the header on CCSR

In CCSR the opacheader preference isn't really in the header it's to the
right of the search box (see attached). I'm okay with a customizable
region over there, but we should have a new preference for that and
header should be above the search box as it implies.

To test:
1) Put something in the OpacHeader syspref. Preferably something large
   and header-like for maximum effect.
2) Select CCSR theme.
3) View OPAC. Notice placement of "header" content.
4) Apply patch.
5) View OPAC. Notice placement of "header" content.
6) Sign off.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, works as advertised with one caveat; the content of
opacheader is by default underneath the div "top-bar". A couple breaks
in my OpacHeader test made it visible, so I'm going to assume this is a
styling issue and not a bug as such.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Comment 16 Galen Charlton 2013-06-14 23:08:03 UTC Comment hidden (obsolete)
Comment 17 Galen Charlton 2013-06-14 23:09:38 UTC
I've set the status back to needs-signoff, but please note that it's my follow-up patch that is need of additional review.
Comment 18 Chris Cormack 2013-06-23 07:10:35 UTC
Created attachment 19198 [details] [review]
Bug 8884: (follow-up) rearrange placement of the top-bar div in CCSR

This patch makes it easier to avoid having the top bar
overlap other OPAC page content (particularly custom
header content set via the opacheader system preference).

This is done by moving the top-nav div outside of the
main doc3 div and adjusting the CSS accordingly.  Yes,
this does end up having top-nav.inc be included from
opac-bottom.inc, but it would require touching rather
more templates to have it be included immediately after
the <body> tag.

This patch also removes a duplicate instance of the
cmspan span.

To test:

[1] Change to the CCSR OPAC theme.
[2] Before applying the patch, open a set of OPAC
    pages (e.g., main page, user page, search results, bib
    details) in your web browser.
[3] Apply the patch, and open the same OPAC pages in separate
    tabs.  Comparing the pre- and post- version of each page
    to verify that the placement
[4] Add an OpAC header using the opacheader system preference.
[5] Verify that the header is displayed without overlapping
    the top bar.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 19 Galen Charlton 2013-06-30 15:34:48 UTC
Pushed to master along with the follow-up.  Thanks, Jared!
Comment 20 Tomás Cohen Arazi 2013-07-15 18:09:59 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Jared!
Comment 21 Bernardo Gonzalez Kriegel 2013-07-16 17:40:50 UTC
First patch pushed to 3.10.x, will be in 3.10.8
Second does not apply.