Bug 8660 - Tag status does not show on multiple tag add
Summary: Tag status does not show on multiple tag add
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-21 09:37 UTC by Fridolin Somers
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

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


Attachments
Proposed patch (41.31 KB, patch)
2012-08-22 14:37 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (2) (41.31 KB, patch)
2012-08-23 07:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (3) (39.54 KB, patch)
2012-08-23 07:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8660: Tag status does not show on multiple tag add (39.62 KB, patch)
2012-08-23 13:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8860 follow-up: Porting opac.css changes to cssr theme (1.78 KB, patch)
2012-10-02 14:42 UTC, Paul Poulain
Details | Diff | Splinter Review
Proposed patch (rebased master) (39.83 KB, patch)
2012-10-18 10:26 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up) (rewritten) (5.20 KB, patch)
2012-10-18 10:29 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 8660: follow-up: Porting changes to cssr theme (5.30 KB, patch)
2012-11-01 07:58 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2012-08-21 09:37:29 UTC
On OPAC search results, when adding tag on selected records, nothing changes.
Status should appear on selected lines.
Comment 1 Fridolin Somers 2012-08-22 14:37:42 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-08-22 19:10:41 UTC
I really like the improvements in this patch, but I also have a complaint: Whitespace changes in otherwise unchanged lines makes it difficult to track what is really different and what isn't. I would prefer that whitespace-only changes be submitted in a separate patch if they're really desirable.

I see only one glitch in functionality that needs to be addressed:

If I choose some items (in results, the cart, or a list) and add a tag, I see the "tag added" message by each one. If I then select some different items on that same list and add another tag, the "tag added" message on the original selection doesn't disappear altogether, it just loses its text, leaving an empty yellow box behind.

It's not a big issue, but I would like to see a revision before I sign off. Thanks!
Comment 3 Chris Cormack 2012-08-22 20:55:56 UTC
(In reply to comment #2)
> I really like the improvements in this patch, but I also have a complaint:
> Whitespace changes in otherwise unchanged lines makes it difficult to track
> what is really different and what isn't. I would prefer that whitespace-only
> changes be submitted in a separate patch if they're really desirable.
> 
I agree, speaking as a release maintainer, separate patches make my life (and anyone ever doing code maintenance in the future) a lot easier.
Comment 4 Fridolin Somers 2012-08-23 07:08:57 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2012-08-23 07:29:01 UTC
(In reply to comment #2)
> 
> I see only one glitch in functionality that needs to be addressed:
> 
> If I choose some items (in results, the cart, or a list) and add a tag, I
> see the "tag added" message by each one. If I then select some different
> items on that same list and add another tag, the "tag added" message on the
> original selection doesn't disappear altogether, it just loses its text,
> leaving an empty yellow box behind.
> 
> It's not a big issue, but I would like to see a revision before I sign off.
> Thanks!
Your right, corrected in new patch (a span with border-radius must be hidden when empty)
Thanks a lot for testing.
Comment 6 Fridolin Somers 2012-08-23 07:40:51 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2012-08-23 13:45:09 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2012-10-02 14:38:17 UTC
QA comment:
 * patch signed-off since more than 1 month, QAing it myself
 * template only changes, low side effect risk
 * koha-qa.pl is OK:
 ../qa/koha-qa.pl -v 2 -c 1

testing 1 commit(s) (applied to commit 813c744)
 * 8b19523 Bug 8660: Tag status does not show on multiple tag add
      koha-tmpl/opac-tmpl/prog/en/css/opac.css
      koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc
      koha-tmpl/opac-tmpl/prog/en/js/tags.js
      koha-tmpl/opac-tmpl/prog/en/modules/opac-basket.tt
      koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
      koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
      koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
      koha-tmpl/opac-tmpl/prog/en/modules/opac-suggestions.tt

    - koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc                  OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK
    - koha-tmpl/opac-tmpl/prog/en/modules/opac-basket.tt                       OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK
    - koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt                       OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK
    - koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt                      OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK
    - koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt                      OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK
    - koha-tmpl/opac-tmpl/prog/en/modules/opac-suggestions.tt                  OK
	forbidden patterns          OK
	tt_valid                    OK
	valid_template              OK

  * some css added to opac/prog, I added them in opac/ccsr as well


Testing the feature (using the CCSR theme)
 * when I add tag to multiple biblios, I see the status on each line
 * when I add a tag to a given biblio, from the biblio list, It does not work anymore= clicking on "Add" does nothing (no message, no change, no javascript error)
Comment 9 Paul Poulain 2012-10-02 14:42:45 UTC Comment hidden (obsolete)
Comment 10 Fridolin Somers 2012-10-18 10:26:34 UTC
Created attachment 12905 [details] [review]
Proposed patch (rebased master)

Main patch rebased
Comment 11 Fridolin Somers 2012-10-18 10:29:39 UTC Comment hidden (obsolete)
Comment 12 Mason James 2012-11-01 07:58:12 UTC
Created attachment 13168 [details] [review]
Bug 8660: follow-up: Porting changes to cssr theme

Signed-off-by: Mason James <mtj@kohaaloha.com>
Comment 13 Mason James 2012-11-01 08:00:54 UTC
(In reply to comment #12)
> Created attachment 13168 [details] [review]
> Bug 8660: follow-up: Porting changes to cssr theme
> 
> Signed-off-by: Mason James <mtj@kohaaloha.com>

i tested this patch with the CSSR theme, it works well :)
Comment 14 Jared Camins-Esakov 2012-11-01 19:33:21 UTC
String changes. Will hold for post-3.10.0.
Comment 15 Jonathan Druart 2012-11-19 14:32:57 UTC
QA Comments:

Great and clean patch.

Note: I did not test the CCSR theme.

Marked as Passed QA.
Comment 16 Jared Camins-Esakov 2012-11-23 21:47:48 UTC
This patch has been pushed to master.
Comment 17 Chris Cormack 2012-11-26 06:40:15 UTC
Pushed to 3.10.x will be in 3.10.1
Comment 18 Sophie MEYNIEUX 2013-02-18 10:35:52 UTC
Could it be also in 3.8.x ?
Comment 19 Chris Cormack 2013-02-18 20:33:03 UTC
Doesn't apply to 3.8.x if it could be rebased and signed off then yep :)