Bug 172379 - Automatically expand search result and make the result keyboard accessible
Summary: Automatically expand search result and make the result keyboard accessible
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 2.0-SVN
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 175762 177954 193480 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-10-08 11:17 UTC by Carsten Schlipf
Modified: 2009-12-09 11:34 UTC (History)
10 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
a patch for this bug, and a couple of others too... (4.53 KB, patch)
2009-05-18 04:11 UTC, Tuomas Nurmi
Details
an updated patch (4.54 KB, patch)
2009-05-19 01:10 UTC, Tuomas Nurmi
Details
third try (6.91 KB, patch)
2009-05-20 00:51 UTC, Tuomas Nurmi
Details
a minor improvement (1.09 KB, patch)
2009-06-15 21:55 UTC, Tuomas Nurmi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carsten Schlipf 2008-10-08 11:17:19 UTC
Version:           1.90 (using 4.1.2 (KDE 4.1.2), Kubuntu packages)
Compiler:          gcc
OS:                Linux (i686) release 2.6.24-19-generic

I am really missing the quick search of the old amarok, although on the left of the collection tab there is also such a search field. But it has some problems:

Search results are collapsed. If I enter a title, I first have to expand the results to see the title. But I am entering the search using the keyboard and it is annoying to have to use the mouse and expand the artist and the album to finally see the result. So I need two additional clicks on every single item to see the result. This really bugs me (just my personal opinion, but since I heavily used the quick search on Amarok 1.4 this is the ultimate reason for me to not switch to the new Amarok).

The result is not accessible via keyboad. In Amarok 1.4 I simply used the down arrow to go to the desired match, hit enter and it started to play. In Amarok 2 a couple of mouse clicks are required here.
Comment 1 Casey Link 2008-10-14 03:01:47 UTC
Changing from a wish to normal bug..
Comment 2 Casey Link 2008-10-14 05:15:44 UTC
When return is pressed in the search widget, immediately filter; in the colleciton tree view, upon a filterNow, 
grab keyboard focus.  Fixes one half of bug 172379.

CCBUG:172379


from Jeff rev 869957
Comment 3 Casey Link 2008-10-14 05:18:01 UTC
This is part of bug 172379. It enables navigation of the CB tree with the arrow keys. There is a bug in Qt that causes the app shortcuts to intercept all keyevents, ignoring context, so the left and right arrows will not work until this bug is patched. I blundered about and couldn't get the filter results to auto expand, so feel free to take a crack at that.
CCBUG:172379 
rev 871173 
Comment 4 Dan Meltzer 2008-12-02 18:01:37 UTC
This is not going to be fixed until after 2.0.0, retargetting.
Comment 5 Clovis Gladstone 2009-03-29 01:04:47 UTC
Is there any estimate as to when this will be fixed ? It keeps being pushed back, and I really would love to see this feature back in amarok. Thanks for your great work.
Comment 6 Tuomas Nurmi 2009-05-18 04:11:22 UTC
Created attachment 33783 [details]
a patch for this bug, and a couple of others too...

Here's a patch, which does two things:
1) Addresses this bug (and #177954 and #175762 too, they seem to be duplicates). When the collection is being filtered and there's not too many results, they get auto-expanded. Pretty similar to Amarok 1.4.
2) Addresses bug #160026 by adding a "Collapse all" button next to collection filter bar, removing the need to click all those minuses as described in that bug report.

Some words on the patch itself:
I used arrow-left icon for the collapse button, it was the most fitting that came to my mind. But there might be a better option...

The description for the collapse button contains a new string, didn't find any fitting from the existing ones.

I disabled the separate automatic expanding of various artists, as it behaved totally different from others. Now it acts pretty much like the others. But I commented out only one line, I'm not sure if some other lines there are useful any more. Someone familiar with VA functionality might want to check if there's something more to comment out in case this patch is commited...


The patch pretty much works for me (but might kill your kitten).
Comment 7 Myriam Schweingruber 2009-05-18 13:16:18 UTC
*** Bug 175762 has been marked as a duplicate of this bug. ***
Comment 8 Myriam Schweingruber 2009-05-18 13:16:49 UTC
*** Bug 177954 has been marked as a duplicate of this bug. ***
Comment 9 Tuomas Nurmi 2009-05-18 22:32:08 UTC
Hmm, seems like the patch makes Magnatunes browser behave a bit weird, seeing if I find a solution...
Comment 10 Jeff Mitchell 2009-05-18 23:18:58 UTC
Makes the Magnatune browser "behave a bit weird" in what way?
Comment 11 Tuomas Nurmi 2009-05-19 01:10:33 UTC
Created attachment 33820 [details]
an updated patch

Okay, updated patch. The last version expanded all the visible genres in Magnatunes instead of a single one, fixed now. Also, simplified the code a bit.
Comment 12 Jeff Mitchell 2009-05-19 16:52:22 UTC
Two problems:

1) Keyboard doesn't actually work for me.  Pressing down/up don't do anything.

2) If you clear the search box, all albums are shown, except an expand is then triggered on the albums that were expanded earlier + just found, which can cause the collection browser to be a huge mess of expanded albums.
Comment 13 Tuomas Nurmi 2009-05-19 19:50:33 UTC
1) Oh, indeed. I don't use keyboard in collection browser so didn't notice. Will fix later today.

2) I believe it behaves pretty equal to Amarok 1.4 there (correct me if I'm wrong). In bug 160026's comments Seb says this is intended behaviour. But the patch also adds the collapse all -button to upper right of the collection browser for easy collapsing.
Comment 14 Tuomas Nurmi 2009-05-20 00:51:29 UTC
Created attachment 33848 [details]
third try

Added support for up & down keys. Hopefully the way I did it is sensible...
Comment 15 Seb Ruiz 2009-05-21 13:32:45 UTC
SVN commit 970917 by seb:

Automatically expand search result and make the result keyboard
accessible. Thanks to the excellent patch by Tuomas Nurmi.

NB: I've cleaned up the patch, and I dropped the "collapse all" button,
since it neither worked very well, nor did it look great. Let's work on
this separately.

BUG: 172379
CCMAIL: tuomas@norsumanageri.org

 M  +3 -0      ChangeLog  
 M  +7 -10     src/browsers/CollectionTreeItemModelBase.cpp  
 M  +1 -0      src/browsers/CollectionTreeItemModelBase.h  
 M  +34 -0     src/browsers/CollectionTreeView.cpp  
 M  +2 -0      src/browsers/CollectionTreeView.h  
 M  +7 -0      src/widgets/LineEdit.cpp  
 M  +6 -0      src/widgets/LineEdit.h  
 M  +2 -0      src/widgets/SearchWidget.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=970917
Comment 16 Seb Ruiz 2009-05-21 13:50:38 UTC
*** Bug 193480 has been marked as a duplicate of this bug. ***
Comment 17 Mark Kretschmann 2009-05-21 13:57:20 UTC
The automatic expanding on filtering is very nice, thanks for that :)

But I have an idea for improving it further: When you clear the filter, the expanded items should collapse again.


I think that would make it even better. What do you think, Tuomas?
Comment 18 Carsten Schlipf 2009-05-21 14:57:05 UTC
Mark, that sounds like a great idea.
Comment 19 Jeff Mitchell 2009-05-21 15:32:14 UTC
Mark, I mentioned that above in comment #12 (and this is the main reason I didn't commit this patch).  Tuomas said that this is the way it was in 1.4, but that doesn't seem a great reason (to me) to keep it this way -- I find it annoying to not have them collapse again, as it clutters up the collection browser.  I think this needs to be fixed before release, or this patch reverted for now.
Comment 20 Nikolaj Hald Nielsen 2009-05-21 18:58:56 UTC
Please revert this one! It completely breaks searching in Magnatune, Jamendo and the Podcast Directory services!

Besides, we are in feature freeze for a reason, lets hold this one as a nice little extra for 2.1.1.
Comment 21 Jeff Mitchell 2009-05-21 19:45:42 UTC
SVN commit 971119 by mitchell:

Reverting per Nikolaj's very reasonable request that we're in feature freeze, and that this demonstrably breaks things.  Can try again for 2.1.1.

CCBUG: 172379


 M  +0 -3      ChangeLog  
 M  +10 -7     src/browsers/CollectionTreeItemModelBase.cpp  
 M  +0 -1      src/browsers/CollectionTreeItemModelBase.h  
 M  +0 -34     src/browsers/CollectionTreeView.cpp  
 M  +0 -2      src/browsers/CollectionTreeView.h  
 M  +0 -7      src/widgets/LineEdit.cpp  
 M  +0 -6      src/widgets/LineEdit.h  
 M  +0 -2      src/widgets/SearchWidget.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=971119
Comment 22 Tuomas Nurmi 2009-05-21 20:27:08 UTC
comment #20: strange, searching in at least Magnatune and Podcast directory seem to work for me (or then I just don't realize what's broken)


the automatic collapse: sure, if it's the desired behaviour now. Will try doing that :)
Comment 23 Tuomas Nurmi 2009-05-22 00:10:50 UTC
Hmm, actually, in svn revision 970917 they don't work for me, but they certainly did when I was using some older revision with the latest patch. Haven't yet found out why's that. But I'll investigate it further after I've finished the AutoCollapse-O-Mat...
Comment 24 Seb Ruiz 2009-06-14 09:00:27 UTC
SVN commit 981691 by seb:

Reapply commit 970917
BUG: 172379

 M  +3 -0      ChangeLog  
 M  +7 -10     src/browsers/CollectionTreeItemModelBase.cpp  
 M  +1 -0      src/browsers/CollectionTreeItemModelBase.h  
 M  +34 -0     src/browsers/CollectionTreeView.cpp  
 M  +2 -0      src/browsers/CollectionTreeView.h  
 M  +13 -0     src/widgets/LineEdit.cpp  
 M  +6 -0      src/widgets/LineEdit.h  
 M  +2 -0      src/widgets/SearchWidget.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=981691
Comment 25 Tuomas Nurmi 2009-06-15 21:55:09 UTC
Created attachment 34558 [details]
a minor improvement

Somewhy I just can't seem to get the auto collapsing right, so I'm giving up on working it (for now?).

But, attached patch should clean the expanding behaviour for various artists a bit by running the autoexpand only once and not separately after compilation and normal queries...
Comment 26 Gábor Lehel 2009-09-05 16:36:46 UTC
"Mark, I mentioned that above in comment #12 (and this is the main reason I
didn't commit this patch).  Tuomas said that this is the way it was in 1.4, but
that doesn't seem a great reason (to me) to keep it this way -- I find it
annoying to not have them collapse again, as it clutters up the collection
browser."

Being the person who implemented much of this behaviour for 1.x, I think I can authoritatively state that the reason it worked that way was a side effect of the implementation, and not specifically intentional. I think the reason I didn't "fix" it was a combination of not seeing it as very significant, and that if you expand some items manually after filtering, those would've also ended up collapsed after removing the filter, unless you add a whole 'nother layer of complication to the implementation.

I'm not sure how 2.x works currently, but the ideal behaviour would be to separately keep track of items the user expanded manually and ones that were expanded automatically by Amarok, and preserve the former throughout adding and removing filters, but collapse the latter automatically when removing filters. Dunno how difficult that would be with 2.x. (Would've been rather nontrivial with 1.x, iirc.)