Bug 11312 - Add span to facets (for css selection)
Summary: Add span to facets (for css selection)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 11538 11546 (view as bug list)
Depends on: 11105 11106
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-27 15:26 UTC by Jonathan Druart
Modified: 2014-12-07 20:07 UTC (History)
5 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 11312: Facets should be selectable using an id [staff] (2.88 KB, patch)
2013-11-27 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-prog] (2.82 KB, patch)
2013-11-27 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-bootstrap] (2.97 KB, patch)
2013-11-27 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [staff] (2.88 KB, patch)
2014-01-14 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-prog] (2.89 KB, patch)
2014-01-14 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-bootstrap] (2.97 KB, patch)
2014-01-14 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [staff] (2.88 KB, patch)
2014-01-14 20:26 UTC, Francesca
Details | Diff | Splinter Review
signed-off Bug 11312: Facets should be selectable using an id [staff] (2.95 KB, patch)
2014-01-14 20:29 UTC, Francesca
Details | Diff | Splinter Review
signed-off Bug 11312: Facets should be selectable using an id [opac-prog] (2.95 KB, patch)
2014-01-14 20:31 UTC, Francesca
Details | Diff | Splinter Review
signed-off Bug 11312: Facets should be selectable using an id [opac-bootstrap] (3.04 KB, patch)
2014-01-14 20:32 UTC, Francesca
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [staff] (2.94 KB, patch)
2014-01-15 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-prog] (2.95 KB, patch)
2014-01-15 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: facets labels are not css selectable -- opac-bootstrap (3.35 KB, patch)
2014-01-15 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-bootstrap] (3.29 KB, patch)
2014-01-15 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [staff] (2.94 KB, patch)
2014-01-16 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-prog] (2.95 KB, patch)
2014-01-16 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: facets labels are not css selectable -- opac-bootstrap (3.35 KB, patch)
2014-01-16 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-bootstrap] (3.18 KB, patch)
2014-01-16 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [staff] (2.99 KB, patch)
2014-01-17 20:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-prog] (3.00 KB, patch)
2014-01-17 20:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11312: facets labels are not css selectable -- opac-bootstrap (3.40 KB, patch)
2014-01-17 20:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11312: Facets should be selectable using an id [opac-bootstrap] (3.23 KB, patch)
2014-01-17 20:11 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-11-27 15:26:14 UTC
Bug 11106, bug 8989, bug 11105 add an id for facets.
It should be similar for staff and opac (prog and bootstrap).
Comment 1 Jonathan Druart 2013-11-27 15:37:15 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-11-27 15:37:21 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-11-27 15:37:25 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-01-14 15:33:22 UTC
*** Bug 11538 has been marked as a duplicate of this bug. ***
Comment 5 Jonathan Druart 2014-01-14 15:33:49 UTC
*** Bug 11546 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Druart 2014-01-14 15:36:28 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-01-14 15:36:37 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2014-01-14 15:36:43 UTC Comment hidden (obsolete)
Comment 9 Francesca 2014-01-14 20:26:59 UTC Comment hidden (obsolete)
Comment 10 Francesca 2014-01-14 20:29:56 UTC Comment hidden (obsolete)
Comment 11 Francesca 2014-01-14 20:31:03 UTC Comment hidden (obsolete)
Comment 12 Francesca 2014-01-14 20:32:17 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-01-15 09:04:49 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2014-01-15 09:04:54 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-01-15 09:04:59 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-01-15 09:05:09 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2014-01-15 09:07:14 UTC
I added the Francesca's patch (from bug 11546).
Comment 18 Katrin Fischer 2014-01-15 20:47:07 UTC
I tested and saw some small issues that could be fixed in a follow up. As we don't have bootstrap in 3.12 the boostrap follow up would be best as a separate patch.

INTRANET & PROG OPAC
The span for the author facet is not closing correctly:
+[% IF facets_loo.type_label_Authors %]<span id="facet-authors"Authors</span>[% END %]

Also it would be nice, if all use were following the same naming schema. Most ids use a minus, but we have 2 using an underscore instead.

BOOTSTRAP
The HTML structure here is a bit different than for the other 2 themes. The patch adds a span inside a h5 tag. It would be cleaner, if we changed it, so the h5 tag has the id. In order to do that the <h5> before and the </h5> after the facet block can be deleted and the span has to be changed to h5. The result would look someting like this:

[% IF ( facets_loo.type_label_Topics ) %]<h5 id="facets-Topics">Topics</h5>[% END %]

We could also standardize the naming scheme a bit more, by using all lowercase ids. This way it would be the same ids for all 3 templates.
Comment 19 Jonathan Druart 2014-01-16 08:18:12 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2014-01-16 08:18:17 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2014-01-16 08:18:25 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-01-16 08:18:33 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-01-16 08:19:30 UTC
(In reply to Katrin Fischer from comment #18)

> INTRANET & PROG OPAC
> The span for the author facet is not closing correctly

Hum... (/me is hidden)

> Also it would be nice, if all use were following the same naming schema.
> Most ids use a minus, but we have 2 using an underscore instead.

Yes of course! I don't know what I did...

> BOOTSTRAP
> The HTML structure here is a bit different than for the other 2 themes. The
> patch adds a span inside a h5 tag. It would be cleaner, if we changed it, so
> the h5 tag has the id. In order to do that the <h5> before and the </h5>
> after the facet block can be deleted and the span has to be changed to h5.
> The result would look someting like this:
> 
> [% IF ( facets_loo.type_label_Topics ) %]<h5
> id="facets-Topics">Topics</h5>[% END %]
> 
> We could also standardize the naming scheme a bit more, by using all
> lowercase ids. This way it would be the same ids for all 3 templates.

In fact I forgot to re-upload a patch for the bootstrap theme. It is why the code was completely different. I replaced the span with h5.
Comment 24 Kyle M Hall 2014-01-17 20:10:54 UTC
Created attachment 24531 [details] [review]
Bug 11312: Facets should be selectable using an id [staff]

Test plan:
launch a search and verify all facet headers have an id.

Signed-off-by: Francesca Moore <francescalamoore@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2014-01-17 20:11:17 UTC
Created attachment 24532 [details] [review]
Bug 11312: Facets should be selectable using an id [opac-prog]

Signed-off-by: Francesca Moore <francescalamoore@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2014-01-17 20:11:20 UTC
Created attachment 24533 [details] [review]
Bug 11312: facets labels are not css selectable -- opac-bootstrap

To test:
1. search in OPAC directory
2. inspect the facets, note that they now have spans

Signed-off-by: David Cook <dcook@prosentient.com.au>

Works as described.

I'm not sure that this change is necessary, since the parent li elements
have IDs that can be hooked onto for CSS, but it doesn't change existing
behaviour and more control is always nice, so that's all right.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Kyle M Hall 2014-01-17 20:11:23 UTC
Created attachment 24534 [details] [review]
Bug 11312: Facets should be selectable using an id [opac-bootstrap]

Signed-off-by: Francesca Moore <francescalamoore@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Galen Charlton 2014-01-23 16:58:38 UTC
Pushed to master.  Thanks, Francesca and Jonathan!