-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Country list in edit.js #563
Conversation
<option value="CK">Cook Islands</option> | ||
<option value="CR">Costa Rica</option> | ||
<option value="CI">Côte d'Ivoire</option> | ||
<option value="HR">Croatia</option> | ||
<option value="CU">Cuba</option> | ||
<option value="CW">Curaçao</option> | ||
<option value="CY">Cyprus</option> | ||
<option value="CZ">Czech Republic</option> | ||
<option value="CZ">Czechia</option> | ||
<option value="CD">Democratic Republic of the Congo</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be Congo? Having "Democratic" at the front would change the alphabetic ordering and might make it harder to find?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full name of this country is Democratic Republic of the Congo, not to be confused with (Republic of the) Congo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah interesting. Cool, let's keep it as is!
src/views/profile/edit.ejs
Outdated
<option value="CK">Cook Islands</option> | ||
<option value="CR">Costa Rica</option> | ||
<option value="CI">Côte d'Ivoire</option> | ||
<option value="HR">Croatia</option> | ||
<option value="CU">Cuba</option> | ||
<option value="CW">Curaçao</option> | ||
<option value="CY">Cyprus</option> | ||
<option value="CZ">Czech Republic</option> | ||
<option value="CZ">Czechia</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Czech Republic more official?
https://en.wikipedia.org/wiki/Czech_Republic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both are acceptable, but I believe ISO has it listed as Czechia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, could we keep it as Czech Republic then? Just anecdotally that's what I've been more familiar with. But if you think Czechia is more common/official let's go with that. I'll leave it up to you :).
Just thinking from a usability perspective and what's easiest for users.
src/views/profile/edit.ejs
Outdated
@@ -164,14 +165,14 @@ | |||
<option value="GY">Guyana</option> | |||
<option value="HT">Haiti</option> | |||
<option value="HM">Heard Island and McDonald Islands</option> | |||
<option value="VA">Holy See (Vatican City State)</option> | |||
<option value="VA">Holy See</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I kinda liked having the bracket here lol. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose the bracket could be readded
src/views/profile/edit.ejs
Outdated
@@ -247,57 +248,58 @@ | |||
<option value="PT">Portugal</option> | |||
<option value="PR">Puerto Rico</option> | |||
<option value="QA">Qatar</option> | |||
<option value="CG">Republic of the Congo</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, looks like we have 3 Congos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should remove this one and keep Congo imo. DRC is a different country.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
<option value="BQ">Bonaire, Sint Eustatius and Saba</option> | ||
<option value="BA">Bosnia and Herzegovina</option> | ||
<option value="BW">Botswana</option> | ||
<option value="BV">Bouvet Island</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't have an addition, did you mean to completely remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops
@@ -184,11 +185,10 @@ | |||
<option value="KZ">Kazakhstan</option> | |||
<option value="KE">Kenya</option> | |||
<option value="KI">Kiribati</option> | |||
<option value="KP">Korea, Democratic People's Republic of</option> | |||
<option value="KR">Korea, Republic of</option> | |||
<option value="XK">Kosovo</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks new. Not sure if we have a flag icon for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. There is a new one. I'll get a flag when i can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I did a bit of a sad thing and also had the list copied here:
https://github.com/vck3000/ProAvalon/blob/master/src/routes/profile.js#L403-L907
Could you update this list after you finish up the changes here?
Maybe you could pull that list out into a separate ts file and have it be pulled in so that they share the same source of truth. If that's too much effort dw about it and just update the list to match haha.
Thanks :).
No description provided.