KhalisFoundation / sttm-desktop

The SikhiToTheMax Desktop App
Open Software License 3.0
34 stars 35 forks source link

Long-term fix for custom BG #407

Closed maneetpaul closed 5 years ago

maneetpaul commented 5 years ago

Ticket to track a long-term fix for custom BG file duplication. Also, when you change from a custom theme to a regular theme, and then back to the custom theme, the image needs to be re-uploaded.

inderpreetsingh commented 5 years ago
tsingh777 commented 5 years ago

So the issue right now is when we "upload" an image we copy it to <userDataPath>/user_backgrounds we aren't reducing the size so essentially we are copying the image to another location on the disk. the plugins for imagemin module weren't working with the built version of the application and were removed to get the feature out. Copy the file is not the best solution in my opinion.

Some options:

Both of these options would require a "default" image incase the user ever deletes the file and would just indicate to them they need to re-upload and delete the "saved" image.

maneetpaul commented 5 years ago

Something to keep in mind - there is a high chance that people will be deleting images off of the desktop if it is a Gurdwara laptop. My suggestion: allow people to upload an image as-is (which will create a copy of it). However, when somebody deletes it from the new "Recent Backgrounds" section, it will also delete the duplicated file.

inderpreetsingh commented 5 years ago

@tsingh777, aah I see. Earlier I was taking the image directly from where the file was placed and was requested to change that to incorporate imagemin. I like the idea of a symlink, I will explore it.

However, now that user can delete those custom background images, and also can see list of images she has uploaded right there in the interface, I don't think having a couple of duplicate images is such a bad thing. I am thinking of a scenario where the user chooses a custom background and then deletes the image, having forgotten that it's being used in sttm. πŸ€·β€β™‚οΈ Yes, now the user will instead see a default background image but i personally would prefer a user experience where sttm saves that image and user can delete it from the sttm interface if she wants to. Any deletion/modification of that image outside sttm imho should not effect sttm version of the image. πŸ˜† if that makes sense!

inderpreetsingh commented 5 years ago

@maneetpaul that's what happens right now, you remove from recent backgrounds and it deletes the duplicate file. πŸ™

maneetpaul commented 5 years ago

I think we are in good shape then with what you have implemented.

@tsingh777 let us know if you strongly disagree with this approach or if we can move this forward!

tsingh777 commented 5 years ago

prefer a user experience where sttm saves that image and user can delete it from the sttm interface if she wants to. Any deletion/modification of that image outside sttm imho should not effect sttm version of the image.

If that's what we are going for, then sounds good. Should a limit to the number of custom backgrounds be enforced?

maneetpaul commented 5 years ago

Let’s limit it to five for now and see what kind of feedback we get (this is just an arbitrary number I picked, I think we should keep it under 10)

Also, can we add text for recommended image size of 1920x1080 near the upload button?