[dolphin/search] Search by (multiple) tags
ClosedPublic

Authored by iasensio on Nov 3 2019, 5:11 PM.

Details

Summary

Adds a tag selector in the extended filters of the search box.
Selected tag or tags are added to the search query along with the other filters (type, date, rating).

FEATURE: 412564
CCBUG: 356062

Test Plan
  • Menu shows the user tags
  • Picking any tag/s filters the search to that specific tag/s

Diff Detail

Repository
R318 Dolphin
Branch
search_tags
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19609
Build 19627: arc lint + arc unit
iasensio created this revision.Nov 3 2019, 5:11 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptNov 3 2019, 5:11 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
iasensio requested review of this revision.Nov 3 2019, 5:11 PM

This looks fantastic!

iasensio updated this revision to Diff 69418.Nov 8 2019, 1:11 AM
  • Use tag:<tag> token
  • Fix style
iasensio planned changes to this revision.Nov 13 2019, 12:04 AM

Will have to be rebased after D25260

iasensio updated this revision to Diff 70590.Nov 29 2019, 7:47 PM
  • Rebase
  • Add test case for tags
iasensio edited the summary of this revision. (Show Details)Nov 29 2019, 7:48 PM

Nice, this looks great and works great. Only one real suggestion: disable it when there are no tags. Otherwise clicking on it does nothing.

src/search/dolphinfacetswidget.cpp
70

Just tag is fine; the *-symbolic icon names are generally for compatibility with GNOME apps.

iasensio updated this revision to Diff 70595.Nov 30 2019, 12:21 AM
iasensio marked an inline comment as done.
iasensio edited the summary of this revision. (Show Details)
  • Use tag icon

Only one real suggestion: disable it when there are no tags. Otherwise clicking on it does nothing.

Nice idea! Baloo indexer is failing for me now due to some non-related assert error, so I cannot delete the tags to test it, but will try again.

iasensio updated this revision to Diff 70696.Dec 1 2019, 9:36 PM

Disable selector when there are no tags

ngraham accepted this revision as: VDG, ngraham.Dec 2 2019, 2:54 AM

UI looks good to me.

This revision is now accepted and ready to land.Dec 2 2019, 2:54 AM
elvisangelaccio requested changes to this revision.Dec 8 2019, 6:14 PM
elvisangelaccio added inline comments.
src/search/dolphinfacetswidget.cpp
99

this-> not needed.

262

this-> not needed

283

Missing reference usage

src/search/dolphinfacetswidget.h
29

Not needed

32

Not needed

90

Initialization not needed, the defautl ctor is automatically called.

This revision now requires changes to proceed.Dec 8 2019, 6:14 PM
iasensio updated this revision to Diff 71108.Dec 9 2019, 5:49 AM
iasensio marked 6 inline comments as done.

Address comments

iasensio updated this revision to Diff 71109.Dec 9 2019, 5:52 AM
  • Fix indentation
iasensio updated this revision to Diff 71403.Dec 12 2019, 11:10 PM

Clarify updateTagSelector logic

elvisangelaccio accepted this revision.Dec 15 2019, 12:29 PM
This revision is now accepted and ready to land.Dec 15 2019, 12:29 PM
This revision was automatically updated to reflect the committed changes.
bruns added a subscriber: bruns.Dec 15 2019, 4:47 PM
bruns added inline comments.
src/search/dolphinfacetswidget.cpp
146

method name no longer fits

150

comment no longer fits

169

dito

iasensio added inline comments.Dec 15 2019, 4:53 PM
src/search/dolphinfacetswidget.cpp
146

I totally agree.
I'm planning a clean-up refactor patch for this methods, and to also remove the "lazy" split-parsing it tries to do, which hasn't been needed for a while.