Allow permanantly saving images (Requires #146) #147
Conversation
There was a problem hiding this comment.
Thanks for the implementation! The changes are mostly fine; I only have a few comments.
I didn't test these changes out right now. Is there a way to set an image from the saved favorites as wallpaper? As far as I understood, you only copy them to the favorites folder, but there is no change to the user interface except the 'save' button and the settings UI. Is that right?
| if (favoritesFolder === "") { | ||
| const directoryPictures = GLib.get_user_special_dir(GLib.UserDirectory.DIRECTORY_PICTURES); | ||
|
|
||
| if (directoryPictures === null) { | ||
| // Pictures not set up | ||
| const directoryDownloads = GLib.get_user_special_dir(GLib.UserDirectory.DIRECTORY_DOWNLOAD); | ||
|
|
||
| if (directoryDownloads === null) { | ||
| const xdg_data_home = GLib.get_user_data_dir(); | ||
| favoritesFolder = Gio.File.new_for_path(xdg_data_home); | ||
| } else { | ||
| favoritesFolder = Gio.File.new_for_path(directoryDownloads); | ||
| } | ||
| } else { | ||
| favoritesFolder = Gio.File.new_for_path(directoryPictures); | ||
| } |
There was a problem hiding this comment.
I don't think that this fallback chain is the best idea. It could lead to leaving pictures in unexpected places. I.e., we can not easily predict where the images will be stored on a given system when no path was set up manually.
Instead, I'd rather create a folder next to this.wallpaperlocation as the fallback. So basically here: ${xdg_cache_home}/${Self.metadata['uuid']}/favourites.
There was a problem hiding this comment.
I agree, although the cache folder is for data that can be regenerated and thus is unsuited.
Do you have a strong preference to one folder that's already included in this ugly chain?
$HOME/Pictures/…$HOME/Downloads/…$XDG_DATA_HOME/…
Another possibility would be to disable the button if no folder was set manually.
There was a problem hiding this comment.
[...] is for data that can be regenerated and thus is unsuited.
Hm
From those folders, I'd use $XDG_DATA_HOME, to be honest. I generally don't want to "pollute" personal directories of people without their permission (I hate it when software creates folders in random places that I expect to "own" :D). You just have to take care of the case that $XDG_DATA_HOME might not be set on all distributions.
Another possibility would be to disable the button if no folder was set manually.
This is also a valid option. We could also just hide the button entirely when no folder is set. Or alternatively, directly open the file chooser for the storage folder when none is set.
I just want to make sure, that we do not store the images in random places. I added an issue here so we can merge this PR for now.
Yes, this is just a "I want to keep this image for later"-Button, maybe the favorite naming in the code is a bit misleading. It is (or will be) named 'Save For Later' on the button itself. Having the same folder added as a local folder source can mimic a favorite image behavior. |
That is a good idea; we might want to make a shortcut for that in the UI and maybe add a dedicated "Favourites Source" which is just a local source in the background. Also added an issue for that here. |
I added the ability to save images for later. This fixes #16 and probably #86 ?
Together with the local folder source it's now possible to have favorites.