Bug 8339 - div id="login" should be id="login_controls" in header.inc
Summary: div id="login" should be id="login_controls" in header.inc
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low minor
Assignee: Mark Tompsett
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-30 08:19 UTC by Mark Tompsett
Modified: 2013-12-05 20:01 UTC (History)
3 users (show)

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


Attachments
changed from login to login_controls in header.inc and staff-global.css (1.41 KB, patch)
2012-06-30 14:00 UTC, Mark Tompsett
Details | Diff | Splinter Review
Changed login to login_controls in header.inc and patched staff-global.css (1.81 KB, patch)
2012-09-04 07:17 UTC, Mark Tompsett
Details | Diff | Splinter Review
Renamed login to login_controls in header.inc and staff-global.css (1.51 KB, patch)
2012-09-11 17:07 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8339 - div id="login" should be id="login_controls" in header.inc (1.61 KB, patch)
2012-09-11 18:12 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 Mark Tompsett 2012-06-30 08:19:53 UTC
If one wishes to customize the login screen of the staff client, CSS changes to div#login also affect the control set in the top menu bar. The top menu bar needs a different id name.
Comment 1 Mark Tompsett 2012-06-30 14:00:53 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-06-30 19:56:46 UTC
You only have to correct the English files, all other languages are created from those files and the translations in the po files.
Comment 3 Jared Camins-Esakov 2012-07-02 17:53:36 UTC
The name is fine, but the change in id breaks the header layout once you are logged in on Chrome under Mac. The alignment is off.
Comment 4 Mark Tompsett 2012-09-04 07:17:06 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2012-09-10 19:12:59 UTC
(In reply to comment #4)
> This should not cause any strange repositioning of the control set in the
> header div on the right. It may not be pixel identical, but generally should
> not look vastly different.

It doesn't have to look different at all! Instead of adding a new definition for #login_controls, change the ID of the original declaration for #login at line 161:

#login {
	position : absolute;
	right : .5em;
}

Change #login to #login_controls.

The Koha login form will be unaffected because there is a separate stylesheet which defines #login for that page.
Comment 6 Mark Tompsett 2012-09-10 19:24:20 UTC
The problem is the way cascading works. Renaming in place won't work. It needs to be below the ".clearfix" class. I was struggling for hours when I renamed in place, all because of the .clearfix class. I could put it in front of the #login_controls at the top if I wanted it in place, but it made more sense to be located with the other class that shares the header div.
Comment 7 Mark Tompsett 2012-09-10 19:28:23 UTC
And to follow up... because it is before the .clearfix class definition. The current #login unpatched does nothing, as far as I can tell.
Comment 8 Mark Tompsett 2012-09-11 17:07:32 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2012-09-11 18:12:42 UTC
Created attachment 12146 [details] [review]
[SIGNED-OFF] Bug 8339 - div id="login" should be id="login_controls" in header.inc

After experiencing what I believe was stale CSS, I have
implemented oleonard's suggested fix which does work. That is:
header.inc change login to login_controls, and in
staff-global.css change the #login to #login_controls.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Jonathan Druart 2012-09-12 14:02:35 UTC
The template file modules/auth.tt still use the login id for a div element.
Please clarify if it is normal.

Marked as Failed QA.
Comment 11 Owen Leonard 2012-09-12 14:36:21 UTC
That's exactly the point. It removes the ambiguity between what <div id="login"> refers to. The <div> on auth.tt will now be identified differently than the <div> in the staff client header.
Comment 12 Jonathan Druart 2012-09-12 14:58:14 UTC
(In reply to comment #11)
> That's exactly the point. It removes the ambiguity between what <div
> id="login"> refers to. The <div> on auth.tt will now be identified
> differently than the <div> in the staff client header.

Ok thank you Owen.

So I marked it as Passed QA.
Comment 13 Paul Poulain 2012-09-13 16:36:16 UTC
Patch pushed to master
Comment 14 Chris Cormack 2012-09-13 23:35:29 UTC
Pushed to 3.8.x will be in 3.8.5