Bug 20452 - Add HTML classes to search results availability data
Summary: Add HTML classes to search results availability data
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: paxed
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-20 10:51 UTC by paxed
Modified: 2018-10-02 15:03 UTC (History)
2 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 20452: Add HTML classes to search results availability data (9.97 KB, patch)
2018-03-20 10:54 UTC, paxed
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description paxed 2018-03-20 10:51:16 UTC
The data in the Location-column in the search results table is impossible to style because there are no HTML classes. Sprinkle those around, so libraries can hide information they don't need.
Comment 1 paxed 2018-03-20 10:54:06 UTC
Created attachment 73101 [details] [review]
Bug 20452: Add HTML classes to search results availability data

The data in the Location-column in the search results table is
impossible to style because there are no HTML classes. Sprinkle
those around, so libraries can hide information they don't need.

Test plan:
1) Apply patch
2) Search for biblios
3) Check that the data in the Location-column looks correct.
4) Optionally, look through the page source that it looks correct.
Comment 2 Mark Tompsett 2018-03-20 13:14:00 UTC
While the patch is good, I'm thinking that such short generic names is probably a bad thing. For example, look at the staff-global.css or the opac.less file. There are branchgrid things or #item things. Perhaps just adding the word list would be a good idea?
Comment 3 paxed 2018-03-21 06:25:11 UTC
(In reply to M. Tompsett from comment #2)
> While the patch is good, I'm thinking that such short generic names is
> probably a bad thing. For example, look at the staff-global.css or the
> opac.less file. There are branchgrid things or #item things. Perhaps just
> adding the word list would be a good idea?

The patch doesn't add any branchgrid class, so I don't know why that would be a problem? Also, staff-global.css doesn't have any selectors for #item? (And the patch doesn't add any item class either - it has itemcn, which is not matched by anything in staff-global.css - and certainly shouldn't be!

If there were any css selectors that match globally with a short, generic, simple word commonly used in Koha (say, for example. "item"), then those selectors should be written to be more specific - For example, a hypothetical:

#item { background: pink; }

should be written to match only at the place where we want that pink background, say in the search results:

#searchresults #item { background: pink; }

Otherwise, the too widely matching selector would have side effects, for example sometime later, people might add code that uses class item, but aren't expecting it to show up with pink bg.
Comment 4 Mark Tompsett 2018-03-21 12:06:56 UTC
You missed my point. I think the class name is bad. Add 'list' to it, because otherwise, someone else for something else will add 'item' and then wonder why it doesn't work the first time.
Comment 5 Mark Tompsett 2018-03-21 12:10:00 UTC
(In reply to paxed from comment #3)
> The patch doesn't add any branchgrid class, so I don't know why that would
> be a problem? Also, staff-global.css doesn't have any selectors for #item?
> (And the patch doesn't add any item class either - it has itemcn, which is
> not matched by anything in staff-global.css - and certainly shouldn't be!

I was giving examples of how longer names were already used, rather than just "branch", "item", etc.
Comment 6 paxed 2018-03-21 12:16:03 UTC
(In reply to M. Tompsett from comment #4)
> You missed my point. I think the class name is bad. Add 'list' to it,
> because otherwise, someone else for something else will add 'item' and then
> wonder why it doesn't work the first time.

Which class name? Prepend "list" to all of those new classes?
Comment 7 Katrin Fischer 2018-03-25 22:00:32 UTC
Maybe better to use 'results_'. Would this be ok for you?
Comment 8 Mark Tompsett 2018-03-26 01:55:34 UTC
(In reply to Katrin Fischer from comment #7)
> Maybe better to use 'results_'. Would this be ok for you?

Don't care what is prepended, appended... just not so bare as "branch", "item", etc. and as long as it makes sense.
Comment 9 Owen Leonard 2018-10-02 15:03:45 UTC
(In reply to Katrin Fischer from comment #7)
> Maybe better to use 'results_'. Would this be ok for you?

I agree with this suggestion.