State display & ISO 3166 Abbreviations

[ScottFries]

Adding State to location display and abbreviating State & Country displays (state only applies to USA & CAN states currently).

The existing display of <City, Country> was ambiguous to me (we Americans love reusing City names in every state :) ), so this change aims to allow including State while also allowing State and Country to be abbreviated to ISO 3166 values.

@JW-CH would love your input on this to make sure I'm not coming at this with too much of a USA/Canada focus. I've only added State-abbreviation logic for those two countries and my understanding is that the only EU state that has "states" is Germany, and I believe all of the other EU states will simply not show any field for "State" as was the case before (and Germany will display a non-abbreviated state currently). @3rob3 I'm not sure what part of the world you're at, but your input is always welcome as well!

[3rob3]

Adding State to location display and abbreviating State & Country displays (state only applies to USA & CAN states currently).

The existing display of <City, Country> was ambiguous to me (we Americans love reusing City names in every state :) ), so this change aims to allow including State while also allowing State and Country to be abbreviated to ISO 3166 values.

@JW-CH would love your input on this to make sure I'm not coming at this with too much of a USA/Canada focus. I've only added State-abbreviation logic for those two countries and my understanding is that the only EU state that has "states" is Germany, and I believe all of the other EU states will simply not show any field for "State" as was the case before (and Germany will display a non-abbreviated state currently). @3rob3 I'm not sure what part of the world you're at, but your input is always welcome as well!

I'm USA also (Ohio). For Jan's (@JW-CH) context, when we see a photo location from U.S. it can be quite verbose like: California, United States of America That takes up a lot of screen real estate. I'd personally be ok just showing City name, but for those who travel abroad more often maybe that's not ok.

[ScottFries]

Adding State to location display and abbreviating State & Country displays (state only applies to USA & CAN states currently). The existing display of <City, Country> was ambiguous to me (we Americans love reusing City names in every state :) ), so this change aims to allow including State while also allowing State and Country to be abbreviated to ISO 3166 values. @JW-CH would love your input on this to make sure I'm not coming at this with too much of a USA/Canada focus. I've only added State-abbreviation logic for those two countries and my understanding is that the only EU state that has "states" is Germany, and I believe all of the other EU states will simply not show any field for "State" as was the case before (and Germany will display a non-abbreviated state currently). @3rob3 I'm not sure what part of the world you're at, but your input is always welcome as well!

I'm USA also (Ohio). For Jan's (@JW-CH) context, when we see a photo location from U.S. it can be quite verbose like: California, United States of America That takes up a lot of screen real estate. I'd personally be ok just showing City name, but for those who travel abroad more often maybe that's not ok.

Definitely agree that "United States of America" is way too space-heavy, which was the main reason for me wanting to the the abbreviations part of this change. Personally I probably would've dropped it entirely, but I do have some photos on display from Canada as well and will likely be going over seas in the near future too. Was likely going to add a toggle setting for country display as well though for those that don't care about it in general.

[ScottFries]

Unrelated, but saw that you invited me as a collaborator @3rob3; not sure if you deleted that or if everything is just against me collaborating with y'all, but I'm getting a "Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account." following that invite and still can't join the Discord. Don't really know of a resolution to either of those; just don't want you thinking I'm trying to dodge comms; would love for both of those links to work already! 🤣

[3rob3]

Unrelated, but saw that you invited me as a collaborator @3rob3; not sure if you deleted that or if everything is just against me collaborating with y'all, but I'm getting a "Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account." following that invite and still can't join the Discord. Don't really know of a resolution to either of those; just don't want you thinking I'm trying to dodge comms; would love for both of those links to work already! 🤣

oops, my bad. I added you but must have cancelled or deleted. I re-requested.

As for Discord, maybe you need to join the main Immich Discord first, then ImmichFrame is a channel on there. try this: https://discord.com/invite/jXKruJCr6M

[JW-CH]

@ScottFries Thank you for your change and the request for my input.

Germany is not the only EU-Country that uses states, for example in Switzerland (where I live) we have so called 'cantons' that are basically what you are referring to states. Other countries have something similar.

As you already mentioned, we don't really use the states in displaying a location as Americans do because everything is more narrow spacewise in the EU, but I totally see your point. I just skimmed over your pr-draft and wanted to mention that I dislike the 'hard coded' states and I assume there has to be a solution for this already. How does immich do something like this? Do they even display that somewhere, or do they just have the standard EXIF-Info? Make it configurable? Or just display the state in addition when the EXIF-Info is US/CA? But I like the clean code and the change :) Thank you

[ScottFries]

@ScottFries Thank you for your change and the request for my input.

Germany is not the only EU-Country that uses states, for example in Switzerland (where I live) we have so called 'cantons' that are basically what you are referring to states. Other countries have something similar.

As you already mentioned, we don't really use the states in displaying a location as Americans do because everything is more narrow spacewise in the EU, but I totally see your point. I just skimmed over your pr-draft and wanted to mention that I dislike the 'hard coded' states and I assume there has to be a solution for this already. How does immich do something like this? Do they even display that somewhere, or do they just have the standard EXIF-Info? Make it configurable? Or just display the state in addition when the EXIF-Info is US/CA? But I like the clean code and the change :) Thank you

Thanks for the input! I agree that the hard-coded dictionary for USA and CAN states is far from ideal, but I haven't been able to find a preexisting solution that handles these (System.Globalization and the ISO 3166 NuGet package @3rob3 mentioned above handles countries, but nothing more local than that and there doesn't seem to be a solution for states searching through NuGet).

As far as how Immich handles this (as far as I'm aware), they just pull the standard City, State, Country info from the EXIF-Info and display it in full which as we've mentioned tends to be too verbose (e.g. "San Jose, California, United States of America" while most people would expect "San Jose, CA, USA", potentially even omitting the ", USA"). I did start with just adding the full-text state, but that made the length even more bothersome, though admittedly just abbreviating the country alone goes a long way in reducing this.

[ScottFries]

Any preference on where to go from here for now @3rob3? Short of making my own ISO-3166 package (which I'm kinda tempted to do tbh) I'm not seeing a path to eliminate the manual dictionary creation. If that's too controversial for the main repo atm I can split out the country abbreviation part of this change.

[3rob3]

Any preference on where to go from here for now @3rob3? Short of making my own ISO-3166 package (which I'm kinda tempted to do tbh) I'm not seeing a path to eliminate the manual dictionary creation. If that's too controversial for the main repo atm I can split out the country abbreviation part of this change.

I’m torn TBH. The current dictionary you have is fine, I just don’t know where we go from there? It could get out of hand having to add those for the rest of the world, but also doesn’t seem right to have this a US only feature. Is there any compromise that works? Thinking along the lines of “city, abbreviated country”, “city, state, abbreviated country”, “city” only? Any of those via configuration? I guess what I’m saying is I would prefer to keep it the same for all regions, therefore my vote would be only abbreviate the country (since this seems doable), and make each field configurable.

Just so we’re clear, I would also love abbreviated state so don’t think I’m shitting on your idea.

[ScottFries]

Any preference on where to go from here for now @3rob3? Short of making my own ISO-3166 package (which I'm kinda tempted to do tbh) I'm not seeing a path to eliminate the manual dictionary creation. If that's too controversial for the main repo atm I can split out the country abbreviation part of this change.

I’m torn TBH. The current dictionary you have is fine, I just don’t know where we go from there? It could get out of hand having to add those for the rest of the world, but also doesn’t seem right to have this a US only feature. Is there any compromise that works? Thinking along the lines of “city, abbreviated country”, “city, state, abbreviated country”, “city” only? Any of those via configuration? I guess what I’m saying is I would prefer to keep it the same for all regions, therefore my vote would be only abbreviate the country (since this seems doable), and make each field configurable.

Just so we’re clear, I would also love abbreviated state so don’t think I’m shitting on your idea.

"so don’t think I’m shitting on your idea." Didn't take it that way at all, and it's no skin off my back either way; at the end of the day this is gonna be the change my fork has that I use either way. I just wanna make sure the parks that make sense to have in mainline that are useful for others makes it there!

Seems like we all agree that while there's nothing inherently wrong with this approach, it's far from elegant and leaves support to be desired (regardless of if that's due to missing implementations and/or different regional standards). Because of that, I'm going to pull the state abbreviations part for now (feel free to point people to my fork if they're asking for that), bring in the country abbreviations (without adding another NuGet package for the time being since this can be done with System packages), and bring in the option to display state (while adding config toggles for each of the 3 fields).

[3rob3]

Sounds good! Appreciate you contributing!

[3rob3]

Looks good. I would like to make a suggestion to simplify things a bit. Instead of having individual Settings and checkboxes for Show City, Show State, Show Country how about a single drop-down that binds to a single string Setting FormattedLocation. Then just pattern match against that before returning in your GetLocationString method? The drop-down would be: "City" "City, State" "City, State, Country" You could also bind isEnabled of it to ShowImageLocation.

I'd also suggest getting rid of AbbreviateCountry setting and just do it by default. I can't imagine anyone objecting to that. Feel free to object though.

Thoughts? The amount of settings were already getting crazy and this seems like a good way to simplify.

[ScottFries]

Looks good. I would like to make a suggestion to simplify things a bit. Instead of having individual Settings and checkboxes for Show City, Show State, Show Country how about a single drop-down that binds to a single string Setting FormattedLocation. Then just pattern match against that before returning in your GetLocationString method? The drop-down would be: "City" "City, State" "City, State, Country" You could also bind isEnabled of it to ShowImageLocation.

I'd also suggest getting rid of AbbreviateCountry setting and just do it by default. I can't imagine anyone objecting to that. Feel free to object though.

Thoughts? The amount of settings were already getting crazy and this seems like a good way to simplify.

Ya, that makes sense. I was already considering making an Enum mask to support this or a multi-checkbox selection. Will get something together to clean up. As far as the mandating abbreviated countries goes, my cut reaction was that some people might not want this if they're not familiar with the ISO-3166 abbreviations (e.g. I had no idea Switzerland was CE). If I go with the mask approach I might leave the option in since it'll be "free", but will see how this goes.