Closes T7271
Details
- Reviewers
mart ngraham - Group Reviewers
Plasma - Maniphest Tasks
- T7271: File Search
- Commits
- R119:d66bd8f3e9af: WIP: Beginning of the Baloo/Search KCM Rewrite
Diff Detail
- Repository
- R119 Plasma Desktop
- Branch
- kcm_baloo_qml
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 16652 Build 16670: arc lint + arc unit
kcms/baloo/package/ui/main.qml | ||
---|---|---|
34 ↗ | (On Diff #65378) | should use a Kirigami.FormLayout to have checkboxes centered? |
kcms/baloo/package/ui/main.qml | ||
---|---|---|
32 ↗ | (On Diff #65378) | While we're at it, let's improve this string: "This module lets you configure the system-wide file search features" |
- Fix metadata installation
- Fix model loading on Qml
- Base work on the Qml
- Name fixes
- Move ui/main to contents
- Fix wrong types in Qml
- Plug the controllers
- Link to the save button
Getting there!
Use an actual framed list view for the list of folders to not index, like the current one has.
FWIW, it would be good to fix some of the bugs listed here as part of the port: https://bugs.kde.org/buglist.cgi?component=kcm_baloo&list_id=1663634&product=systemsettings&resolution=---
This isn't ready yet without, at a minimum, the UI change I requested for the list view.
- Fix metadata installation
- Fix model loading on Qml
- Base work on the Qml
- Name fixes
- Move ui/main to contents
- Fix wrong types in Qml
- Plug the controllers
- Link to the save button
- Fix borders and background for the Baloo KCM
kcms/baloo/package/metadata.desktop | ||
---|---|---|
4 ↗ | (On Diff #65378) | That comment sounds wrong :) |
The frame's alignment and padding seem off:
Could we use a list more like the one used in the Virtual Desktops KCM and the slideshow wallpaper folder list? Then we would have a unified appearance (Goal: Consistency :) ).
Yeah, use a Kirigami BasicListItem.
kcms/baloo/package/contents/ui/main.qml | ||
---|---|---|
2 ↗ | (On Diff #66643) | Shouldn't this be you copyright? |
- Fix metadata installation
- Fix model loading on Qml
- Base work on the Qml
- Name fixes
- Move ui/main to contents
- Fix wrong types in Qml
- Plug the controllers
- Link to the save button
- Fix borders and background for the Baloo KCM
- Use kirigami BasicListItem
- Spacing
- Fix mapping of roles
- Fix delegate / Mouse integration
See inline comments.
Also functionality-wise there are some problems:
- When a new entry is added, if the KCM is closed and re-opened, the new entry has been forgotten
- When you add a path, the ugly URL schema bit doesn't get stripped off like it should:
kcms/baloo/filteredfoldermodel.cpp | ||
---|---|---|
148 ↗ | (On Diff #65378) | Don't use Q_FOREACH in new or ported code |
kcms/baloo/package/contents/ui/main.qml | ||
2 ↗ | (On Diff #66643) | Not done yet |
23 ↗ | (On Diff #67563) | IIRC plasma isn't depending on Qt 5.12 yet, right? |
39 ↗ | (On Diff #67563) | This is a sentence, so it needs to end with a period. |
47 ↗ | (On Diff #67563) | This can be simplified to just if (!checked) |
50 ↗ | (On Diff #67563) | Don't need to reference it with fileSearchEnabled, just do kcm.indexing = checked |
53 ↗ | (On Diff #67563) | Don't do it this way, just use a binding: checked: kcm.indexing |
69 ↗ | (On Diff #67563) | Same comments as with the first checkbox |
71 ↗ | (On Diff #67563) | I would add some spacing between the last checkbox and the list view's explanatory label: Item { Layout.preferredHeight: Kirigami.Units.gridUnit } |
kcms/baloo/package/metadata.desktop | ||
4 ↗ | (On Diff #65378) | Not done yet |
- Simplify Checkboxes
- Fix previouslyEnabled logic
- Tell Qml that we have the values
- Move code around
- Fix QSet math
- Qml gives us a Url, we want the Local file
- use folderAdded and folderRemoved
- Remove debug information
- Add spacing
I still need to play a bit with the folder save / load. the code from the old kcm is quite strange. it saves "included folders", but there's nothing using that. perhaps it's safer if we just remove everything that's not used.
Almost done, just a tiny bit of polish is needed! See inline comments:
kcms/baloo/kcm.cpp | ||
---|---|---|
59 ↗ | (On Diff #65378) | Make this match the name shown in the list ("File Search") |
kcms/baloo/package/contents/ui/main.qml | ||
32 ↗ | (On Diff #67568) | "This module lets you configure the file indexer and search functionality." |
36 ↗ | (On Diff #67568) | This item isn't inside a Layout, so Layout.<anything> has no effect. You want anchors.<whatever> instead. Also the margins should be Kirigami.Units.largeSpacing. |
LGTM! Would be nice to fix some of these bugs in follow-up patches too: https://bugs.kde.org/buglist.cgi?component=kcm_baloo&list_id=1669138&product=systemsettings&resolution=---
kcms/baloo/package/contents/ui/main.qml | ||
---|---|---|
27 ↗ | (On Diff #67701) | Is this actually used? Also, because it's not installed by default it shows an error. |
kcms/baloo/package/contents/ui/main.qml | ||
---|---|---|
27 ↗ | (On Diff #67701) |