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.
Details
- Reviewers
hein mart - Group Reviewers
Plasma VDG - Commits
- R119:a1fbeb96dc6c: [Colors KCM] Add search and filter
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
Lint Skipped - Unit
Unit Tests Skipped
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)
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
Alright, will change it to a ComboBox. We might lose the filter icon, which I quite liked, in the process, though.
- 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
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.
kcms/colors/package/contents/ui/main.qml | ||
---|---|---|
129 | why do we need a different approach from the way you've done the clear button? |
kcms/colors/package/contents/ui/main.qml | ||
---|---|---|
129 | 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. |
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.
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? |
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..
- Refactor to use proper QAbstractListModel for colors
- Fix current scheme index following as you filter