[UlfSchmidt]

@Buschel I expected that this would not let you go and that you would therefore take these matters into your own hands. I am ready to test!

[Buschel]

Ys, couldn't keep my fingers off this ... I will do some testing on simulator before I ask for a TF build. 

Could you have a look at the latest TF build which did the search changes discussed earlier (GlobalSearch keeps section headers and "sort by label")?

[UlfSchmidt]

Could you have a look at the latest TF build which did the search changes discussed earlier (GlobalSearch keeps section headers and "sort by label")?

Will do these tests tomorrow, I promise.
Last weeks were quite hard as I suffered from post Covid-19 fatigue syndrome …

[Buschel]

No worries, and get well soon!

[amasephy]

I took a look at this new TF but I admit it’s been a little difficult interpreting exactly what this change is. Some screenshots posted in the forum at release could be helpful. I think you posted some previews awhile back.

For example I really do not understand what the change of sort search results by ascending label is supposed to do.


As for the other changes to global search there are a few problems I’ve seen.

1) The section headers are causing what I presume to be cell re-use as you call it. The exact dimension of the header on searching will cause that dimension to be added as padding to the first object out of initial view. See screenshot below. Two songs have extra space under them which isn’t normally present.




2) Section headers while search is active don’t float at the top under the search bar like they do outside of a search. This to me is a lost opportunity.


3) Unclear if this is a regression or long time bug but with year searching makes it easier to detect. In my library I have scrolled through to properly cache all items in global search for reference.

If I search “1985” and get maybe 50 items returned. Then scroll to bottom of search results. Then clear search. Now scroll to the top. I get massive stuttering.

Stuttering has been a problem with global search for awhile for me, but ONLY while scrolling up and typically only if all items haven’t been cached yet. Yet here the stutter remains.


Impressive you decided to tackle refactoring the class when it was very clear you did not want to take that on at the moment. Major kudos. 👍🏻

[Buschel]

For example I really do not understand what the change of sort search results by ascending label is supposed to do.

Thanks for pointing this out. Will be giving some details. First, the related PR has screenshots and more background: https://github.com/xbmc/Official-Kodi-Re.../pull/1153
But let me also spend a few words here. The search until now just filtered the list as it came from Kodi, it did not apply any sort of any kind. In "Top 100 Songs" Kodi will respond with a list of songs, sorted by their playcount. In "All Songs" the sorting done by Kodi is something I never understood. When you searched in such a list the order was kept, but only the items with matching search criteria were kept in the list. So, the underlying sort order was kept. For most of the menus the default is anyway sorted by name/label, you should not see any difference.
The point @UlfSchmidt made is valid, I think. If you anyway search for a name of a list item, it makes sense to sort the results by name as well.

1) The section headers are causing what I presume to be cell re-use as you call it. ...

Oops, never saw this. That's why TF builds are great for testing. Yes, looks like a cell re-use problem. This is most likely not a regression, but maybe got a higher probability to show up. I recently had fixed (1152 in review) another cell re-use issue for GlobalSearch which never got reported. GlobalSearch lists are a bit special as they mix all kind of different layouts. Will review this again.

2) Section headers while search is active don’t float at the top under the search bar like they do outside of a search. This to me is a lost opportunity.

Good point. Not sure, if this works with the additional "x Results" on top, but I will try.

3) ... If I search “1985” and get maybe 50 items returned. Then scroll to bottom of search results. Then clear search. Now scroll to the top. I get massive stuttering.

No idea, I will try it on my devices. But I have few hope.

Impressive you decided to tackle refactoring the class when it was very clear you did not want to take that on at the moment. Major kudos. 👍🏻

Future will let me regret this. 😉 Known glitches will be replaced by a new implementation with unknown issues.

The basics work quite nicely now. But I already saw a glitch I need to understand better, and there is a catch I was not taking into account which impacts the usability (you prefer let the view snap into a left position when moving the pane left, and vice versa) in the reworked implementation. And this will require some special case handling -- I hope not to the extent which was coded before.

Let me spend some time on this, I have few days vacation now.

[Buschel]

@amasephy, can you exactly describe how to get to the "section padding issue"? I cannot reproduce this here (GlobalSearch by Art, by Name, search active or not). A short description and/or a video would be great.

@kambala, can you help with a hint where to look for regarding the "sticky section headers"? I am not clear why the stick headers don't work during search. Is this potentially happening as the searchbar is shown on top of the table?

[UlfSchmidt]

@Buschel, thanks for sorting the search results alphabetically!

Since I was not able to find any serious issue with the current test flight for myself, I tried to reproduce the problems reported by @amasephy:

1) The section headers are causing what I presume to be cell re-use as you call it. The exact dimension of the header on searching will cause that dimension to be added as padding to the first object out of initial view. See screenshot below. Two songs have extra space under them which isn’t normally present.

Never reached a point showing this effect, whatever I tried.

2) Section headers while search is active don’t float at the top under the search bar like they do outside of a search. This to me is a lost opportunity.

I second amasephy, this would be helpful.

3) Unclear if this is a regression or long time bug …
Stuttering has been a problem with global search for awhile for me, but ONLY while scrolling up and typically only if all items haven’t been cached yet. Yet here the stutter remains.

No stuttering on my iPad.

But what I am missing and what I would recommend is to scroll back automatically to the top when the user clears a search to start over. Currently, the scroll bar position is kept, but it doesn’t make any sense to remain there imho.

[Buschel]

Regarding the cell's height I reviewed the code, and found something which might cause problems, depending on call order of methods. I did a change for this and will ask for TF build.

[amasephy]

I’ll post a video later. It’s easy to reproduce. I bet it relies on the exact number of movies I have before music tracks show up and without this magic setup you won’t get the issue.

Regarding the rating disappearing because of cell reuse I saw that issue pr on GitHub and immediately tried to reproduce it and nothing I did could make it happen. Must be the same thing, requires a magic mixture to manifest.



The stuttering is really annoying. It mainly happens when you either do the search like I described or scroll way down via the index and then scroll up. Of course scrolling up and down repeatedly on a series of items the stutter goes away so it’s a caching issue.


I agree with UlfSchmidt, scroll back to the top on clearing a search. The position after a search is cleared is entirely arbitrary currently.

[Buschel]

@amasephy, @UlfSchmidt, scrolling back to top after ending search is a single line of code. I would do this as I am seeing this for other apps (e.g. Apple mail or settings): going back to show the inactive searchbar on top.

[Buschel]

First impression from the reworked stack handling.

• https://www.dropbox.com/scl/fi/wuwwh0a25...18e41&dl=0
  
• always bounces both views (left lesser than right)
  
• generally lesser bounce amplitude to make it more subtle
  
• now bounces also when opening new views
  
• the reported issues after fullscreen (weird bounce and view positions) seem resolved
  
• behaves a bit different as the old one, e.g.
  • dragging right will move the top view to the right border, uncovering the underlying view on the left
    
  • dragging left will move the dragged view to leftmost position and show fully, the right view uncovered and partially shown
    

[UlfSchmidt]

First impression from the reworked stack handling.

Fantastic job! 👍

[amasephy]

Here’s the video showcasing cell re-use issue.


Looking forward to testing the refactored class. Does it only affect iPad app or is it shared with iPhone in some capacity?


Here’s a video of the global search stutter. First I demonstrate scrolling down and back up. No stutter. Then I show the search and clearing search then scrolling back up and massive stutter.

[UlfSchmidt]

You must be very sensitive about stuttering. Even from your video I barely see any stuttering. But I am also a person who doesn’t recognize the often discussed differences between mobile phone displays running at 60 Hz compared to these using 120 Hz…