search

immichFrame / ImmichFrame

GNU General Public License v3.0
469 stars 20 forks source link

Add remote control navigation to Settings screen Closes #91 #95

Closed 3rob3 closed 3 months ago

3rob3 commented 4 months ago

Closes #91

JW-CH commented 4 months ago

Could you also add a Settings.examlpe.json? Also make sure that the Settings.json does not get pushed (Gitignore)!

JW-CH commented 4 months ago

I did not manage to test it in time, sorry. But it looks solid after skipping over the code. I'll be gone for the weekend so I won't be able to fully review.

3rob3 commented 4 months ago

Could you also add a Settings.examlpe.json? Also make sure that the Settings.json does not get pushed (Gitignore)!

Settings.json is just a blank template. It is set to Content so it gets copied to the output folder. If the user doesn't fill it out we will error and allow them to enter via Settings screen, or they can quit and do it manually. If it doesn't exist we will create it. I'm not sure I see a reason for Settings.example anymore.

Edit.....nevermind, you are right. It makes debugging impossible because it overwrites Settings.json each build. I will change it.

3rob3 commented 4 months ago

I did not manage to test it in time, sorry. But it looks solid after skipping over the code. I'll be gone for the weekend so I won't be able to fully review.

Thanks for reviewing. I have been testing it pretty thoroughly.

crobibero commented 4 months ago

This looks like the user would be forced to fully reconfigure the app after update?

3rob3 commented 4 months ago

This looks like the user would be forced to fully reconfigure the app after update?

That has always been the case if not using Settings.xml. This will improve future app updates as the settings should remain now. Also if new settings are added in a future update it will retain your old settings and apply defaults to new ones. I am considering adding a backup/restore option too.

crobibero commented 4 months ago

I installed this on my AndroidTV for testing and can navigate on the error view, but hitting the select button doesn't do anything

3rob3 commented 4 months ago

I installed this on my AndroidTV for testing and can navigate on the error view, but hitting the select button doesn't do anything

Did you build it yourself? This is still in a PR and hasn't been released yet. Should be sometime this week.

crobibero commented 4 months ago

Yes, I built main with this branch merged in to test the functionality

3rob3 commented 4 months ago

Yes, I built main with this branch merged in to test the functionality

ughhh. I'm seeing same thing. I had this working.

3rob3 commented 4 months ago

I reached out to Avalonia team and found out full support for Android TV remotes is not yet released in stable but will be in the next version. There is a RC I can use, but this also requires updating the whole project to .Net 8. I recently tried to do this but ran into some weird build issue with just Android. Long story short, I think I know how to fix this, but it is going to take a bit of work.

JW-CH commented 4 months ago

I like the new Settings.json, but we should provide some kind of "migration" from an XML-File to the new JSON. This should not be hard to do since we can just check for an XML-File, load it, save it as JSON-File and delete the old XML-File.

3rob3 commented 4 months ago

I like the new Settings.json, but we should provide some kind of "migration" from an XML-File to the new JSON. This should not be hard to do since we can just check for an XML-File, load it, save it as JSON-File and delete the old XML-File.

That would be nice, just seems like it’s more effort than it’s worth since it’s just a one time thing. My opinion might be off since I have probably recreated that file 100 times now 😀.

FYI I got things working with .NET 8 and updated Avalonia. I will push some more changes soon.