[Colors KCM] Add search and filter
ClosedPublic

Authored by broulik on Feb 1 2019, 3:20 PM.

Details

Summary

This adds a search bar to search for schemes in the list as well as a filter to show only light or dark themes using a heuristic on the theme's window color.

Test Plan

Should the filter be cleared when you install a new theme to avoid it being hidden by a filter?

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Feb 1 2019, 3:20 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 1 2019, 3:20 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Feb 1 2019, 3:20 PM

An alternative would be


which is a lot more obvious than a tiny filter icon. Your decision, also whether it should highlight when filtered or not (it does not in Widget Explorer where I took this from)

ngraham added a subscriber: ngraham.Feb 1 2019, 6:12 PM

An alternative would be

I would prefer this, but with the filter being a real combobox. In other words, I'd like the following changes:

  • Button should not be flat; should draw the button border (we only put flat buttons in toolbars)
  • The button should have a downward-pointing arrow to indicate that it's a combobox
  • The pop-up doesn't need radio buttons bu the menu items
broulik planned changes to this revision.Feb 1 2019, 6:21 PM

Alright, will change it to a ComboBox. We might lose the filter icon, which I quite liked, in the process, though.

broulik updated this revision to Diff 50667.Feb 1 2019, 6:49 PM
broulik edited the test plan for this revision. (Show Details)
  • Change filter into a simple ComboBox, simplifies code a lot
  • Change placeholder to "Search..." to be consistent with other KCMs
  • Fixed left-to-right layout issues
broulik edited the test plan for this revision. (Show Details)Feb 1 2019, 6:49 PM
broulik updated this revision to Diff 50838.Feb 4 2019, 12:29 PM
broulik edited the test plan for this revision. (Show Details)
  • Add filter icon to ComboBox, needs D18718
ngraham accepted this revision as: VDG.Feb 4 2019, 1:30 PM

UI and interactivity are great. Very nice addition.

Would be nice to have the search field as a re-usable component, yes. As in D18716: Add an ActionTextField component.

davidedmundson added inline comments.
kcms/colors/package/contents/ui/main.qml
138

why do we need a different approach from the way you've done the clear button?

broulik added inline comments.Feb 7 2019, 12:47 PM
kcms/colors/package/contents/ui/main.qml
138

If I could change the left padding of the ComboBox button I would have done that and placed an icon ontop. However, since it's all painted as a monolith by KQuickStyleItem I cannot.

It doesn't follow the padding properties because it's all derived from the QStyle, and changing that opens a can of worm imho.

Is this good now?

UI-wise, I think this can go in.

It does kind of beg the question of why this is the only grid view that has a search, of course.

It does kind of beg the question of why this is the only grid view that has a search, of course.

Dunno, colors has significantly more entries than the other KCMs.

hein accepted this revision.Feb 19 2019, 7:53 AM
hein added a subscriber: hein.

LGTM aside from the minor naming nitpick.

kcms/colors/filterproxymodel.cpp
32

Why did you call this setSourceModelProxy? QSortFilterModel::setSourceModel is virtual, you could do the emit in a reimpl?

This revision is now accepted and ready to land.Feb 19 2019, 7:53 AM
broulik planned changes to this revision.Feb 19 2019, 8:33 AM

The selectedSchemeIndex stuff doesn't take into account the filtering, so the highlight is wrong when filtering and also when marking a theme for deletion the index changes to the next one but might end up applying a currently invisible theme..

broulik updated this revision to Diff 53304.Mar 6 2019, 5:40 PM
  • Refactor to use proper QAbstractListModel for colors
  • Fix current scheme index following as you filter
This revision is now accepted and ready to land.Mar 6 2019, 5:40 PM
broulik requested review of this revision.Mar 6 2019, 5:40 PM
ngraham accepted this revision as: VDG.Mar 6 2019, 7:38 PM
mart accepted this revision.Mar 8 2019, 9:15 AM
This revision is now accepted and ready to land.Mar 8 2019, 9:15 AM
This revision was automatically updated to reflect the committed changes.