Summary: | Automatically expand search result and make the result keyboard accessible | ||
---|---|---|---|
Product: | [Applications] amarok | Reporter: | Carsten Schlipf <carsten.schlipf> |
Component: | general | Assignee: | Amarok Developers <amarok-bugs-dist> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | apoorvkhatreja, chad.calkins, clovisgladstone, hydrogen, illissius, mitchell, nhn, sharpshopter, tuomas, unnamedrambler |
Priority: | NOR | ||
Version: | 2.0-SVN | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: | |||
Attachments: |
a patch for this bug, and a couple of others too...
an updated patch third try a minor improvement |
Description
Carsten Schlipf
2008-10-08 11:17:19 UTC
Changing from a wish to normal bug.. 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 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 This is not going to be fixed until after 2.0.0, retargetting. 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. 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). *** Bug 175762 has been marked as a duplicate of this bug. *** *** Bug 177954 has been marked as a duplicate of this bug. *** Hmm, seems like the patch makes Magnatunes browser behave a bit weird, seeing if I find a solution... Makes the Magnatune browser "behave a bit weird" in what way? 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.
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. 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. Created attachment 33848 [details]
third try
Added support for up & down keys. Hopefully the way I did it is sensible...
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 *** Bug 193480 has been marked as a duplicate of this bug. *** 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? Mark, that sounds like a great idea. 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. 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. 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 #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 :) 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... 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 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...
"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.) |