Thumbnails + current wallpaper

[narfel]

I added the current wallpaper (the file TranscodedWallpaper in C:\Users<user>\AppData\Roaming\Microsoft\Windows\Themes) to the file list because it most often is different from the lockscreen pictures. I also made a panel that list all the pictures in a thumbnail for a quick overview of which image to select.

Detailed list of changes:

• add current wallpaper directory to files array and filter out *.ini file
• skip files < 50kb for lockscreen pictures (hides the xbox thing)
• open second explorer instance on current wallpaper directory
• create a FlowLayoutPanel containing thumbnails of all pictures in their respective orientation to visually see portrait vs landscape
• add some padding and a background color to the selected pictures via a container

[keenthinker]

Hi, thanks for the code changes and the suggestions. I have looked at the code changes and have some comments and suggestions. A few of them I like and a few of them I don't like so much - but that's normal. ;-)

From my point of view, the most important one is that this project was created just for the lock screen images. Now it introduces the desktop images as well. I'm not so keen on adding additional folders and will have to think about it. ;-)

You've changed the styling of the UI a bit - introduced thumbnails and wrapped the picturebox in a panel. In general I like this changes, but personally I still want to see the names of the files - maybe a combination of both would be possible, or two different views. The gray background for the picture box is very subjective. Is it ok for you to leave the wrapper but reduce the padding (e.g. 5 or 10) and leave the background white? ;-)

In my opinion, the logic that checks the directories in “getAssets()” is wrong - you have introduced new directories, but you exit the function if one of them is not available. On my computer, I don't have the “IrisService” folder, I have the other two, but I still get an empty list and the application won't start. The check to see if each directory is present should be closer to each code block populating the list.

When a user clicks on the image to open the image directory, three explorer instances are opened each time. In my personal opinion, this is not user-friendly and helpful - in my case, the third Explorer instance opens my personal directory, as I do not have the target directory. Three separate buttons would be better in this case.

You have removed the configuration of the “statusStrip” button - “DropDownButtonWidth = 0”. Originally this code removed the arrow from the button - now it's still a button, but it looks like a dropdown list. Again, this is not user friendly in my opinion.

I follow the Microsoft C# Coding Style Guidelines (https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names) - private function names are in camelCase - “private Bitmap CreateThumbnail” should be “createThumbnail”. ;-)

Last but not least - please create a feature branch for the pull request - it would make it easier to preview the code changes as you can download and test the branch.

Would you like to make the changes or should I take your code and make the changes?

Many thanks and best regards

[narfel]

Hi, thanks for your considerations. It is my first pull request and I appreciate your reply. Let me first start with the things that are not what I meant to do:
I was under the impression that I did create a feature branch "feature_thumbs" to adress your last point about creating a feature branch. Also I did not realize that my subsequent commit of the IrisService and the removal of the button decoration also counted to the PR. That was meant as a personal customization. I know the IrisService is not on every machine and also your explicit removal of the decoration was clearly a design decision I did not want to change as part of the PR. Just for me personally it wasn't immediately clear that those were buttons so I removed it. Your argument against that is of course equally valid.

Adding additional folders kinda defeat the purpose of an app called WindowsLockScreenImages, I agree. I thought of making an optional path inclusion or add a validation but ultimately decided against it since I just wanted to share my customized code without making sweeping changes or changing the scope. That may or may not be what a PR is for, I'm not sure and I'm sorry if that was the wrong approach.
Since I did not expect the IrisService folder to be in there I decided to not change too much and just duplicated the logic checking the folder. Since there is already hardcoded check for "ContentDeliveryManager_cw5n1h2txyewy" I thought "AppData\Roaming\Microsoft\Windows\Themes" could have enough half-life in it to not check it for existence as well. I was not aware that this blocks execution if it is not there. And again, the IrisService folder never was meant to be in here.

On the other points, like the grey background and the padding my intention was just to emphasize the difference between portrait or landscape and I didn't really think much else. The clunky opening of 3 explorer windows was again just a keep it simple. And of course I agree on the camelCase as well, guilty as charged :)

So all in all, thanks for making this code available and of course feel free to make any changes you want. I'm also happy to do it but I'll probably have to figure out how to roll back that commit and create that feature branch first. Thanks for being gentle with a first timer and best regards

[narfel]

So, upon further reading I think it's not customary to just roll back something, so I just made a new commit removing the IrisService and adding a toggle between thumbnails and file names. I also adressed the other concerns as best I can.
I'm not sure what to do about the feature branch though, I don't think I can create a branch in your repo. On my end it is a branch different from main.

Best regards

[keenthinker]

Hey,

Thanks for the kind reply! I really appreciate it! We were all first-timers at some point, it's just that a lot of people forget that. ;-)

And sorry - I somehow wasn't thinking straight when I wrote my reply regarding the feature branch - the PR comes from your branch, so everything is fine. Sorry for the confusion.

I'll take your code, add a branch and refactor it a bit. There are some features that are really good. I also thought about the desktop images and I think it does fit into the idea of the app. :-)

Let's go!

Many thanks and best regards

[keenthinker]

Thank you for the suggestions and code changes that improve the application.