Bugzilla – Bug 1524
skinnable html for livesearch
Last modified: 2008-12-15 13:06:37 UTC
Livesearch (Slim::Music::Livesearch.pm) contains some hard coded HTML which will work with some skins, but won't with others. And there's some hard coded english text, too...
Change 3158 added translations. Still a lot of hard coded HTML...
I think it will be tricky to take away all the hardcoded stuff. Most of it should be ok (conforming to stylesheets). The graphics are the big problem. If those could be handled through teh stylesheet, then the code could reference a class. This would then be useful for all skins, since those button links get used over and over per skin
I underestimated this one. I thought about storing the records in an array and loop through them in the template. I'll give it a try tomorrow.
Just read the header in Livesearch.pm: # Todo - call filltemplate stuff instead? May be too slow. And is there a reason why it's not using the itemsPerPage setting but its own constant MAXRESULTS?
none of the alpha-sorted browse lists seem to follow itemsPerPage properly anyway. I'm beginning to think it is a useless setting since only browse artwork and browse playlists still adhere to it. browse artwork already hsa a bug request to get rid of items per page, and playlists can easily fit into alpha lists as well. In the case of MAXRESULTS, it seemd to me to make sense becuase you have multiple sets of results from livesearch, so it doesn't work as well with itemsperpage. It might also be that live search is desired to be much faster, so returning only 10 results is fast, followed by a link that simply does an old style search result to grab the rest of the matching tracks. I've never had a case where I have enough artists/albums matching in order to see what happens with those results when it is over MAXRESULTS. now that SN is out, maybe the @slim boys can be let out of the cages to answer some of these questions with definitive answers :)
What was irritating me is that the standard (non-live, hitting the search button) search does respect itemsPerPage. But there are other problems with this mode, too: the number of items found will only be songs, but not the sum of songs, artists and albums. And the pagebar is only paging songs, but not the other items.
What kdf said is correct - itemsPerPage could apply to livesearch, but the default is too large to be reasonable. Also, I'm tired of giving knobs to everyone, when there shouldn't always been knobs. The MAXRESULTS constant was meant to apply to each section in the search: Artist, Album & Tracks.
Dan: what needs to get done here for 6.1?
Dan says that the english text has been removed by Herger, so we now need a patch to use skinnable files.
retitling for what still remains. marking as enhancement since current hard-coding does work for its designed purpose.
Created attachment 2119 [details] make livesearch skinnable I'd like to make the livesearch use templates. Otherwise I'd have to re-implement the search page in JS/JSONRPC for the new Default skin to be anywhere close to the other browse lists.
Created attachment 2120 [details] don't reinvent the wheel - use existing sub routine to fill the result page
I've checked the above patch in, as it's needed for the new skin to work properly Change 12754
This bug appears to have been fixed in the latest release! If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look. Make sure to include the version number of the software you are seeing the error with.