Bug 7876 - Add ids to divs and spans with ids in opac-user.tt
Summary: Add ids to divs and spans with ids in opac-user.tt
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.6
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-02 17:56 UTC by Kyle M Hall
Modified: 2013-12-05 19:59 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:


Attachments
Bug 7876 - Add ids to divs and spans with ids in opac-user.tt (5.89 KB, patch)
2012-04-02 17:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7876 - Add ids to divs and spans with ids in opac-user.tt (5.99 KB, patch)
2012-04-03 22:54 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 7876 - Followup - Fix closing span tag, add fine amount spans. (2.88 KB, patch)
2012-04-04 15:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7876 - Followup - Fix span tag position, add fine amount spans. (3.11 KB, patch)
2012-04-10 16:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7876 - Followup - Fix span tag position, add fine amount spans. (3.23 KB, patch)
2012-04-10 21:16 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-04-02 17:56:37 UTC
Currently, the messages in opac-user.tt cannot be customized via jQuery because the divs do not have ids.
Comment 1 Kyle M Hall 2012-04-02 17:58:58 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-04-03 22:54:29 UTC Comment hidden (obsolete)
Comment 3 Ian Walls 2012-04-04 10:44:50 UTC
Span and Div ids are consistent with variable names, so they are easier to use.  I'm not 100% sure a new tag is exactly required in all cases; some of those list elements could have gotten ids and kept the DOM a little simpler, but that's really not a big deal at all, and always using spans is consistent and simple.

Marking Passed QA.
Comment 4 Paul Poulain 2012-04-04 13:23:38 UTC
unlucky man, you just come to prove that my pre-apply hook works perfectly fine: failed QA because =

koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt contains a tab, must use 4 spaces (+		<span id="renewal_blocked_fines"><li><strong>Please note: </strong> You have <a href="/cgi-bin/koha/opac-account.pl">more than [% renewal_blocked_fines %]</a> in fines. </li></span>)

please fix and resubmit (as passed QA directly)

( thanks because the "warn" in the templates helped me to improve the http://wiki.koha-community.org/wiki/Tips_and_tricks git pre-apply hook )
Comment 5 Paul Poulain 2012-04-04 14:56:47 UTC
OK, with the help of http://blog.bigballofwax.co.nz/2011/12/15/fixing-whitespace-when-apply-patches-with-git/

I've found how to automatically change tabs into 4 spaces:
git config --global core.whitespace trailing-space,space-before-tab,tab-in-indent,tabwidth=4

so, passed QA and pushed !
Comment 6 Kyle M Hall 2012-04-04 15:18:43 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2012-04-04 15:22:00 UTC
I decided to read the diff on my patch and noticed that the closing tag on one the spans for missing its slash ( <span> instead of </span> ). I also noticed that there were no sub-spans for the the fine amount, so that data wouldn't be re-usable with jQuery. Attached is a new patch to resolve those issues. One small bugfix, and one small enhancement.
Comment 8 Chris Cormack 2012-04-07 06:48:59 UTC
Applying: Bug 7876 - Followup - Fix closing span tag, add fine amount spans.
fatal: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7876 - Followup - Fix closing span tag, add fine amount spans.
Comment 9 Kyle M Hall 2012-04-10 16:53:32 UTC Comment hidden (obsolete)
Comment 10 Liz Rea 2012-04-10 21:16:53 UTC
Created attachment 9094 [details] [review]
Bug 7876 - Followup - Fix span tag position, add fine amount spans.

Span was outside, the if statement, could possible cause problems
with jQuery customizations, moved span to inside the conditional.

Also added span for the fine amount itself, so that it may be
re-used for jQuery customizations.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Passed tests, spans appear to be correct.
Comment 11 Ian Walls 2012-04-11 11:13:53 UTC
Template only changing moving <spans> around; marking as Passed QA.
Comment 12 Paul Poulain 2012-04-12 16:11:31 UTC
follow-up pushed (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9094)
Comment 13 Jared Camins-Esakov 2012-05-09 19:58:30 UTC
This bug will be included in the Koha 3.6.5 release.
Comment 14 Jared Camins-Esakov 2012-05-09 20:04:57 UTC
This bug will be included in the Koha 3.6.5 release.