This allows to exlude single images from the slideshow by unchecking the little checkbox in the top right corner.
Feature or todo?: If a folder is removed I don't touch the unchecked Slides. So the list could get very big but if a formerly removed folder is
added again an excluded image will be excluded again.
Details
- Reviewers
ngraham davidedmundson - Group Reviewers
Plasma VDG - Commits
- R120:b1ae890a6ee6: Allow single images to be excluded from the slideshow
- Uncheck some images
- They don't appear in the slideshow
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- toggleSlides
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9478 Build 9496: arc lint + arc unit
This puts permanently disabled checkboxes on the wallpaper delegates that aren't used in the slideshow version, possibly related to a flood of warnings that gets printed to the console:
file:///usr/share/plasma/wallpapers/org.kde.slideshow/contents/ui/WallpaperDelegate.qml:123:13: Unable to assign [undefined] to bool file:///usr/share/plasma/wallpapers/org.kde.slideshow/contents/ui/WallpaperDelegate.qml:123:13: Unable to assign [undefined] to bool file:///usr/share/plasma/wallpapers/org.kde.slideshow/contents/ui/WallpaperDelegate.qml:123:13: Unable to assign [undefined] to bool
Let's not show the checkboxes at all unless we're in the slideshow view.
Also, this has nothing to do with your patch specifically, but seeing so many checkesd checkboxes in the slideshow's preview pane makes me realize just how much I want the Breeze style checkbox to actually look like a checkbox, with a checkmark instead of just a filled in roundrect.
Sorry for that. Actually these warnings were the reason why the checkboxes were visible everywhere. Even if the checkboxes are not visible I got these warnings. In an attempt to get rid of them I tried enabled.
In D18809 I changedthe ListItems to Kirigami SwipeListItems. This changed the remove icon from always visible to visible on hover
Yeah also not a fan of that. I had the idea to give it maybe a small grey rectangle like the action icons as the empty checkboxes or the blue blob can be hard to spot. See the empty checkbox in the last row or the checkbox on Cascade in the screenshot.
If the Breeze checkbox style is presenting problems, we should fix it in Breeze rather than here in downstream users. :)
One thing that might work is the behavior that google photos uses. When you select an item, the item shrinks by about 20% and a blue selection box appears around the image. It just makes it more noticeable that the item has been selected.
Since all items are selected by default here, I'm not sure that behavior would be the most appropriate.
Oh I thought it was a selection. Could the checkbox be located at a top corner sticking out a bit?
It is a selection, it's just that everything is selected by default. :) I think we should try to improve the checkbox's appearance rather than working around its deficiencies IMO.
I tried to do this but the checkbox is cut off. If I move it out if the Rectangle I can't anchor against it because they are not silblings nor is it it's parent. Anchoring to thumbnail also doesn't work.
Anchoring to the whole delegate:
Yeah that doesn't look great with the Breeze style checkbox. Within the frame seems like the way to go, unless we want to take the time to redo Breeze checkboxes first. :)
The checkboxes are right where they're supposed to be, as for their visibility: maybe it would be useful if we had a variable that would allow us to add a background behind a checkbox.
UI looks good now. I've done some code review below:
wallpapers/image/image.cpp | ||
---|---|---|
623 | Don't use Q_FOREACH in new code. See https://www.kdab.com/goodbye-q_foreach/ |