[dolphin/search] Reset search options when needed
Needs ReviewPublic

Authored by iasensio on Sun, Oct 6, 10:40 PM.

Details

Reviewers
elvisangelaccio
meven
ngraham
Group Reviewers
Dolphin
Summary

Adds a method to reset the options in facetsWidget in two cases: when disabled and before parsing a new search URL.
Otherwise, controls for a parameter (ex. rating) which are not found in the new URL would stay on the old positions, instead of the default "any".
See D24422

Test Plan

Controls on facetsWidget go back to the default in such two cases.

Diff Detail

Repository
R318 Dolphin
Branch
search_parse_reset
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17663
Build 17681: arc lint + arc unit
iasensio created this revision.Sun, Oct 6, 10:40 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptSun, Oct 6, 10:40 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
iasensio requested review of this revision.Sun, Oct 6, 10:40 PM
ngraham accepted this revision.Mon, Oct 7, 3:43 PM
This revision is now accepted and ready to land.Mon, Oct 7, 3:43 PM
meven added inline comments.Tue, Oct 8, 7:40 AM
src/search/dolphinfacetswidget.h
54

It is not used yet it seems, I guess you are planning to.

iasensio added inline comments.Tue, Oct 8, 2:07 PM
src/search/dolphinfacetswidget.h
54

It is used in fact in line 111 of dolphinsearchbox.cpp: m_facetsWidget->setEnabled(hasFacetsSupport);, but you are right: this helps a lot the merging of other revisions (and IMHO conceptually makes sense).

elvisangelaccio requested changes to this revision.Sun, Oct 13, 3:42 PM
elvisangelaccio added inline comments.
src/search/dolphinfacetswidget.h
54

QWidget::setEnabled() is not virtual, so we should not create another setEnabled() method in a child class.

You can use the QEvent::EnabledChange event instead: https://doc.qt.io/qt-5/qwidget.html#enabled-prop

This revision now requires changes to proceed.Sun, Oct 13, 3:42 PM
iasensio updated this revision to Diff 67863.Sun, Oct 13, 6:26 PM
  • Use changedEvent() instead of overriding
iasensio marked an inline comment as done.Mon, Oct 14, 2:00 PM