Bug 23837 - detail.pl contains a lot of whitespace in the td.status cell
Summary: detail.pl contains a lot of whitespace in the td.status cell
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-17 19:29 UTC by Christopher Brannon
Modified: 2023-12-18 23:12 UTC (History)
4 users (show)

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


Attachments
Modifying statuses exmple (22.21 KB, image/png)
2023-12-12 19:25 UTC, Christopher Brannon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2019-10-17 19:29:58 UTC
When looking at the code, I am seeing this:



                        

                        

                        

                        

                        

                        
                            
                                Waiting at Athol Library since 10/15/2019.
                            
                            
                                Hold for:
                                 

Not sure why all the whitespace (spaces/new lines/indentation) is there.
Comment 1 Katrin Fischer 2019-10-18 06:06:51 UTC
That will not be the only place where you will see something like this. When writing TT syntax, it leaves 'whitespace' in the code, unless you do something like [%- ... -%].

See:
http://www.template-toolkit.org/docs/manual/Syntax.html#section_Chomping_Whitespace

It will not hurt the output either way. HTML doesn't care about extra spaces.
Comment 2 Christopher Brannon 2019-10-18 17:03:58 UTC
(In reply to Katrin Fischer from comment #1)
> That will not be the only place where you will see something like this. When
> writing TT syntax, it leaves 'whitespace' in the code, unless you do
> something like [%- ... -%].
> 
> See:
> http://www.template-toolkit.org/docs/manual/Syntax.
> html#section_Chomping_Whitespace
> 
> It will not hurt the output either way. HTML doesn't care about extra spaces.

It doesn't hurt the output, until you try to manipulate it with jQuery.  Then it becomes a nightmare.
Comment 3 Katrin Fischer 2019-10-19 15:23:36 UTC
(In reply to Christopher Brannon from comment #2)
> (In reply to Katrin Fischer from comment #1)
> > That will not be the only place where you will see something like this. When
> > writing TT syntax, it leaves 'whitespace' in the code, unless you do
> > something like [%- ... -%].
> > 
> > See:
> > http://www.template-toolkit.org/docs/manual/Syntax.
> > html#section_Chomping_Whitespace
> > 
> > It will not hurt the output either way. HTML doesn't care about extra spaces.
> 
> It doesn't hurt the output, until you try to manipulate it with jQuery. 
> Then it becomes a nightmare.

Let's check that then - what are you trying to do with jQuery?
Comment 4 Katrin Fischer 2023-12-09 11:55:29 UTC
I am pondering if this is a WONTFIX because I don't think we can easily fix it. Adding Owen.
Comment 5 Christopher Brannon 2023-12-12 19:25:20 UTC
Created attachment 159776 [details]
Modifying statuses exmple

Katrin, in response to your question (which I never saw, sorry), we modify the look of the statuses so they stand out better.
Comment 6 Owen Leonard 2023-12-18 17:32:42 UTC
Does this come down to lacking specific classes in the markup to target with jQuery?
Comment 7 Christopher Brannon 2023-12-18 23:12:58 UTC
(In reply to Owen Leonard from comment #6)
> Does this come down to lacking specific classes in the markup to target with
> jQuery?

I think developers have made huge strides in adding the classes that are currently there.  If you saw the attachment above, you can see what we have done to dress up the statuses and make them stand out better for different statuses.  Here is the code I use to achieve this:

  //Item Status Enhancement on detail.pl (v22.05)
  if ($('#catalog_detail').length) {
    $('td.status').wrapInner('<ul class="fa-ul"></ul>');
    //Fix missing status classes (v22.05);
    $('.status span:contains("Available")').addClass('available');
    //END Fix missing status classes
    $('#holdings_table span.available').addClass('label label-success').unwrap().wrap('<h3></h3>');
    $('.status span.holdonitem').wrapInner('<li class="holdonitem"></li>');
    $('.status li.holdonitem').unwrap().prepend('<i class="fa-li fa fa-info-circle"></i>');
    $('.status span.intransit').wrapInner('<li class="intransit" style="color: darkorange;"></li>');
    $('.status li.intransit').unwrap().prepend('<i class="fa-li fa fa-truck"></i>');
    $('.status span.waitingat').wrapInner('<li class="waitingat" style="color: darkorange;"></li>');
    $('.status li.waitingat').unwrap().prepend('<i class="fa-li fa fa-clock-o"></i>');
    $('.status span.heldfor').wrapInner('<li class="heldfor" style="color: darkorange;"></li>');
    $('.status li.heldfor').unwrap().prepend('<i class="fa-li fa fa-caret-right"></i>');
    $('.status li.heldfor').wrap('<ul class="fa-ul"></ul>');
    $('.status li.heldfor').each(function() {
      $(this).parent('ul').next('a').appendTo(this);
    });
    var status_triangle = ['lost','dmg','notforloan','wdn'];
    status_triangle.forEach(function(item) {
      $('.status span.' + item).wrapInner('<li class="' + item + '"></li>');
      $('.status li.' + item).unwrap().prepend('<i class="fa-li fa fa-exclamation-triangle"></i>');
    });
    $('.status span.datedue').wrapInner('<li class="datedue"></li>');
    $('.status li.datedue').unwrap().prepend('<i class="fa-li fa fa-id-card-o"></i>');

    $('.status li.datedue').each(function() {
      var UnwrappedText = $(this).contents().filter(function() {
        return this.nodeType == 3 && this.nodeValue.trim();
      }).wrap('<span class="datedue_text"></span>').end();
      $('span.datedue_text').each(function() {
        var cleantext = $(this).html().replace(/(?:(?:\r\n|\r|\n)\s*)/gm, " ");
        $(this).html(cleantext);
        if (cleantext.includes(': due')) {
          $(this).addClass('patrondue');
        }
      });
    });
    $('.status span.patrondue').wrapInner('<li class="patrondue"></li>');
    $('.status li.patrondue').unwrap().wrap('<ul class="fa-ul"></ul>');
    $('.status li.patrondue').each(function() {
      $(this).text($(this).text().replace(/.: due/g, "Due"));
      $(this).prepend('<i class="fa-li fa fa-caret-right"></i>');
    });
    $('#catalog_detail .shelvingloc:contains("Recently Returned")').each(function() {
      $(this).html($(this).html().replace(/\(Recently Returned\)/g, '<span class="label label-default recentlyreturned">Recently Returned</span>'));
    });
  }
  //END Item Status Enhancement on detail.pl

I think the issue is that there are a lot of elements within the status that are challenging to isolate.  The whitespace is one hurdle, but isolating specific sections of the status remains a challenge.

I think classes to each part would be helpful, but I don't know if I am asking too much.  I do there is enough data being pumped into the single status cell that it would warrant some granular classes.  Again, it just might be me.  I think the end result of what I do works and looks great, but not everyone cares.  I guess it is up to any developer that thinks it is worth adding classes to each section or not.