Fix for bug 300248
Review Request #106325 - Created Sept. 4, 2012 and submitted
Provides a possible improvement to Dolphin's additional search criteria button, according to the request in https://bugs.kde.org/show_bug.cgi?id=300248.
The modified button behaves at it should.
I think "Details" is not an appopriate term. Something like "Refine Search" or so. Note that this can only go into 4.10 due to i18n restrictions in the current branch. (Frank: How can I btw filter for files by keywords? this functionality is mising from the old search sidebar)
Thanks for the patch! About the button text: I'm not sure either what the optimal solution is. Maybe "More Options" and "Hide Options"? Another thing: I was wondering if we should put both the icon and the new text on the button. Maybe this would make it more obvious that this button is different from the others? Just to make sure that noone overlooks the button at first sight. @Kai: I haven't worked on any of the search-related code myself, but it seems that there is indeed not way to search for tags in the new interface. See also https://bugs.kde.org/show_bug.cgi?id=304406
Review request changed
OK, here is a new version, that adds both an the arrow icon and the text. I also changed the condition to set the icon's text to be consistent with the other conditionals in the function.
Revision 2 (+9 -7)
Thanks for the new patch, looks good! And thanks for the hint about the arrow icon, Todd, I agree that being consistent with other apps here is important. One thing that I noticed: maybe the variable 'visible' could get a better name to make it clear at first sight what exactly is visible. Maybe 'advancedOptionsShown' or something like that? Another thing that I noticed: I think it might be better to move the line 'SearchSettings::setShowFacetsWidget(visible)' from DolphinSearchBox::slotFacetsButtonToggled() to DolphinSearchBox::saveSettings(). This would make this more consistent with the other settings and prevent that the settings are saved before the 'showFacetsWidget' setting is changed in the SearchSettings (note that we can't make any assumptions about the order in which the slots invoked by clicking the button are called). Do you agree that these changes make sense? If you have a git account, you can go ahead and push this to master. Otherwise, just upload the final version of the patch here and I'll do it for you. Thanks to everyone who helped to find the best solution for this issue!
Review request changed
Here is another review with the changes you proposed Frank. Please have a look and if it's ok go ahead and push it. I'd also like to try another change but I will need some help. Is there a place we can discuss this or can I just send you a mail? Since I couldn't find a relevant bug/request report for this, I'd like to try it before making it a request.
Revision 3 (+13 -11)
Looks good, thanks! I'll push this the next time I'm at my home computer and have some spare time. > I'd also like to try another change but I will need some help. Is there a place we can discuss this or can I just send you a mail? Just send a message to email@example.com, we can discuss any questions concerning Dolphin's source code there.