This patch splits the single property "Image Size" into "Width" and "Height", providing more fine-tuned control for power users.
FEATURE: 374559
Details
Details
- Reviewers
elvisangelaccio ngraham - Group Reviewers
Dolphin - Commits
- R318:1441fefc07f1: Split Image Size into Width/Height
- Sorting works correctly
- No real change, only exposed differently
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
| src/kitemviews/kfileitemmodel.cpp | ||
|---|---|---|
| 1848–1853 | Since we are splitting into width/height, we can just convert them to int, no? | |
Comment Actions
Merge the ratingRole with widthRole/heightRole
Because the separate roles are now all strings, we can merge them to a single case.
Comment Actions
There are still a few bits of code to be ported:
$ git grep imageSize src/kitemviews/private/kbaloorolesprovider.cpp: if (role == "imageSize") { src/kitemviews/private/kbaloorolesprovider.cpp: // as one string into the "imageSize" role src/kitemviews/private/kbaloorolesprovider.cpp: // width of an image are mapped to the role "imageSize") src/panels/places/placesitemmodel.cpp: props.setVisibleRoles({"text", "imageSize"});
Comment Actions
+1 conceptually; it was never clear to the user how you could sort by both width and height at the same time. Code looks sane now (just one comment), and it works fine in my testing; actually sorting by width or height both seem to work as expected.
| src/kitemviews/kfileitemmodel.cpp | ||
|---|---|---|
| 1850 | Maybe make it explicit that we're falling through for these with`Q_FALLTHROUGH()` | |
| src/kitemviews/kfileitemmodel.cpp | ||
|---|---|---|
| 1850 | TBH, I never heard of Q_FALLTHROUGH() before, so I had to look it up & found this: https://wiki.qt.io/Qt_Coding_Style#Switch_statements Qt advises to not use this if another case follows immediately, but I can obviously still do this if this way is preferred here. | |
| src/kitemviews/kfileitemmodel.cpp | ||
|---|---|---|
| 1850 | Correct, Q_FALLTHROUGH is not supposed to be used here. | |
| src/kitemviews/kfileitemmodel.cpp | ||
|---|---|---|
| 1850 | Hmm, what do you think @elvisangelaccio? | |
| 1850 | Gotcha, thanks for the correction! | |