Bug 24710 - patron-title.inc should have seperate span classes for firstname, othername, surname
Summary: patron-title.inc should have seperate span classes for firstname, othername, ...
Status: RESOLVED DUPLICATE of bug 30568
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-21 19:14 UTC by Lucas Gass
Modified: 2024-02-04 19:38 UTC (History)
11 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 24710: Add span classes to patron first/other/sur names (2.41 KB, patch)
2020-02-21 19:23 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24710: Add span classes to patron first/other/sur names (2.47 KB, patch)
2020-02-21 21:03 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 24710: Add span classes to patron first/other/sur names (2.56 KB, patch)
2020-02-23 19:01 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24710: Add span classes to patron first/other/sur names (2.61 KB, patch)
2020-02-27 04:59 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24710: patron-title.inc should have seperate span classes for names (4.92 KB, patch)
2020-03-03 21:21 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24710: patron-title.inc should have seperate span classes for names (4.99 KB, patch)
2020-03-24 12:49 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2020-02-21 19:14:59 UTC
Some libraries would prefer to be able to customize how the name is displayed in the patron details.

If these included <span> classes we could easily move/hide/reorder bits we don;t want.
Comment 1 Lucas Gass 2020-02-21 19:23:56 UTC
Created attachment 99428 [details] [review]
Bug 24710: Add span classes to patron first/other/sur names

This patch adds some <span> elements with classes to patron-title.inc
TEST PLAN:
1. Have a patron with a firstname, surname, and othername
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .firstname {display: none;}
    .othernames {display:none};
    .surname{display:none}
5. Sign-off
Comment 2 Sarah Cornell 2020-02-21 19:50:55 UTC
Here's our use case:  

We want to display the "other name" value at the end of the patron name because we use it for both additional patron names and for nicknames.  

Current display: 

Jo (Joy Harjo, Sharon Olds, Gertrude Stein, Mary Oliver) Tech (24518000721715)

Ideal display:

Jo Tech (Joy Harjo, Sharon Olds, Gertrude Stein, Mary Oliver) (24518000721715)

I'd test Lucas' patch myself but I'm not good at the IntranetUserCSS syspref yet.

Thanks!
Comment 3 Maryse Simard 2020-02-21 21:03:49 UTC
Created attachment 99451 [details] [review]
Bug 24710: Add span classes to patron first/other/sur names

This patch adds some <span> elements with classes to patron-title.inc
TEST PLAN:
1. Have a patron with a firstname, surname, and othername
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .firstname {display: none;}
    .othernames {display:none};
    .surname{display:none}
5. Sign-off

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 4 Katrin Fischer 2020-02-23 09:56:44 UTC
Hi Lucas, looking at the patch I wonder why some classes seem to be missing:

 [%- IF display_patron_name -%]
     [%- IF data.category_type == 'I' -%]
-        [%- data.surname | html %] [% IF data.othernames %] ([% data.othernames | html %])[% END -%]
+        <span class="surname">[%- data.surname | html %]</span> [% IF data.othernames %] <span class="othernames">([% data.othernames | html %])</span>[% END -%]
     [%- ELSIF invert_name -%]
-        [% data.title | $raw %][%- data.surname | html %], [% data.firstname | html %] [% IF data.othernames %] ([% data.othernames | html %]) [% END -%]
+        [% data.title | $raw %][%- data.surname | html %], <span class="firstname">[% data.firstname | html %]</span> [% IF data.othernames %] ([% data.othernames | html %]) [% END -%]

Here the surname could have a class as well, to keep things consistent.


     [%- ELSE -%]
-        [% data.title | $raw %][%- data.firstname | html %] [% IF data.othernames %] ([% data.othernames | html %]) [% END %] [% data.surname | html -%]
+        [% data.title | $raw %]<span class="firstname">[%- data.firstname | html %]</span> [% IF data.othernames %]<span class="othernames">([% data.othernames | html %])</span> [% END %]<span class="surname"> [% data.surname | html -%]</span>


Should we add a class for title as well?
Comment 5 Lucas Gass 2020-02-23 19:01:02 UTC
Created attachment 99472 [details] [review]
Bug 24710: Add span classes to patron first/other/sur names

This patch adds some <span> elements with classes to patron-title.inc
TEST PLAN:
1. Have a patron with a title, firstname, surname, and othername
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .firstname {display: none;}
    .othernames {display:none};
    .surname{display:none;}
    .title{display:none;}
5. Sign-off
Comment 6 Katrin Fischer 2020-02-24 07:19:42 UTC
Hi Lucas, if you do a follow-up (a second patch with the requested changes) next time, you can keep the signed-off status most of the time (also applies to bug 23547) and the original sign-offer won't be lost either. It's how we usually handle things once the first sign-off has been achieved.
Comment 7 ByWater Sandboxes 2020-02-27 04:59:00 UTC
Created attachment 99683 [details] [review]
Bug 24710: Add span classes to patron first/other/sur names

This patch adds some <span> elements with classes to patron-title.inc
TEST PLAN:
1. Have a patron with a title, firstname, surname, and othername
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .firstname {display: none;}
    .othernames {display:none};
    .surname{display:none;}
    .title{display:none;}
5. Sign-off

Signed-off-by: George Williams <georgew@nekls.org>
Comment 8 Katrin Fischer 2020-02-29 09:30:37 UTC
Hi Lucas,

sorry, but after looking into this more deeply, I saw that I was mistaken to ask for a span around title - we already got one! 

I am sorry for not noticing these things earlier:

1) Remove the title span again, because:

[%- IF data.title %]
    [%- IF no_html %]
        [%- span_start = '' %]
        [%- span_end   = '' %]
    [%- ELSE %]
        [%- span_start = '<span class="patron-title">' %]
        [%- span_end   = '</span>' %]
    [%- END %]
    [% data.title = data.title | html %]
    [%- SET data.title = span_start _ data.title _ span_end _ ' ' -%]

This code already adds a class patron-title. See the no_html flag?

2) Change the other spans to be added the same way, to honor the no_html flag.

I think the no_html is needed because there are some tags that don't allow the use of spans inside. Especially the title tag. Example:

koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt:<title>Koha &rsaquo; Patrons &rsaquo; Purchase suggestions for [% INCLUDE 'patron-title.inc' no_html = 1 %]</title>

3) There is a corresponding file for the OPAC we should change - prefrrably on this bug, but we could also do a separate one:

koha-tmpl/opac-tmpl/bootstrap/en/includes/patron-title.inc
Comment 9 Lucas Gass 2020-03-03 21:21:02 UTC
Created attachment 100078 [details] [review]
Bug 24710: patron-title.inc should have seperate span classes for names

This patch adds some <span> elements with classes to patron-title.inc for both the staff client and OPAC
TEST PLAN:
1. Have a patron with a firstname, surname, and othername and title
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .patron-firstname {display: none;}
    .patron-othernames {display:none};
    .patron-surname{display:none}
    .patron-title{display:none}
5. On the OPAC navigate to /cgi-bin/koha/opac-user.pl and check the names by adding the same CSS to OPACUserCSS
6. Sign-off
Comment 10 PTFS Europe Sandboxes 2020-03-24 12:49:44 UTC
Created attachment 101557 [details] [review]
Bug 24710: patron-title.inc should have seperate span classes for names

This patch adds some <span> elements with classes to patron-title.inc for both the staff client and OPAC
TEST PLAN:
1. Have a patron with a firstname, surname, and othername and title
2. Apply patch
3. There should be no visible changes but if you inscept each element you should see seperate elements with clases.
4. Add each of these to IntranetUserCSS  to make sure it hides that element (this effects patroninfo section AND breadcrumbs)
    .patron-firstname {display: none;}
    .patron-othernames {display:none};
    .patron-surname{display:none}
    .patron-title{display:none}
5. On the OPAC navigate to /cgi-bin/koha/opac-user.pl and check the names by adding the same CSS to OPACUserCSS
6. Sign-off

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 11 Jonathan Druart 2020-03-25 11:01:52 UTC
Lucas, that will not work for all places where patron-title.inc is used, because of the no_html flag.
Not against the approach but, because of the above, I am wondering if this really answer the need.
Comment 12 Jonathan Druart 2020-03-25 11:05:04 UTC
This include file is getting too complex. We should move that the logic to a module.

I am not asking that for now, but that needs to be taken into account for future changes.
Comment 13 Lucas Gass 2020-04-08 14:18:26 UTC
Not sure how I should proceed here, should I go back to something more simple like in the first patch I provided where I simply added <span> classes to the include?
Comment 14 Katrin Fischer 2020-04-08 20:55:05 UTC
(In reply to Lucas Gass from comment #13)
> Not sure how I should proceed here, should I go back to something more
> simple like in the first patch I provided where I simply added <span>
> classes to the include?

The first version doesn't work - it will definitely create problems with the no_html flag.
Comment 15 Katrin Fischer 2020-04-08 20:59:32 UTC
Jonathan, why do you think the latest patch will break for the no_html flag? The latest patch looks good to me on first glance and the OPAC file is not used for a title tag as far as I can tell and is much simpler.
Comment 16 Jonathan Druart 2020-04-15 13:26:28 UTC
When no_html is passed, the spans won't appear and the elements won't be hidden.
Comment 17 Jonathan Druart 2020-04-15 13:28:20 UTC
There are also "\n" displayed in the patron search table.
Comment 18 Jonathan Druart 2020-04-24 10:31:10 UTC
Failing QA regarding the last 2 comments.
Comment 19 Sarah Cornell 2022-01-10 21:39:42 UTC
I'm just stopping by to poke this bug back into life.  We're still interested in reordering title, firstname, surname, and othername values in patron details.  

Not sure how else I can help, though!
Comment 20 Jonathan Druart 2022-01-11 08:15:34 UTC
(In reply to Jonathan Druart from comment #16)
> When no_html is passed, the spans won't appear and the elements won't be
> hidden.

Title elements cannot contain other html tags, and that's why we have the no_html option.

If we want a fully working solution we should have a syspref that defines how patron's names are displayed.
Comment 21 Martin Renvoize 2022-04-27 06:56:03 UTC

*** This bug has been marked as a duplicate of bug 30568 ***
Comment 22 Lucas Gass 2024-02-01 15:44:33 UTC
Could we have 2 includes here? A patron-title-html.inc and a patron-title-nohtml.inc?
Comment 23 Steve, OSLRI, USA 2024-02-04 19:38:38 UTC
We are interested in this bug moving ahead.  We are using "borrowers.othernames" to store what we are calling "legal name if different".  Our primary intent is to serve trans patrons who may not have yet legally changed names.  

We would like this field in the system.  However, we want to dramatically reduce where and how it displays.  E.g., there is no need to see it when simply checking out a book.  In fact, it's really only necessary when confirming ID, pursuing legal action or editing the patron's record.

The ability to more finely control how a name displays would be advantageous to us for this purpose.

Hope this helps!