Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Add a Country dropdown menu list in the Address Editor #24917

Closed
gabrielluong opened this issue Apr 22, 2022 · 9 comments · Fixed by #25236
Closed

Add a Country dropdown menu list in the Address Editor #24917

gabrielluong opened this issue Apr 22, 2022 · 9 comments · Fixed by #25236
Assignees
Labels
eng:qa:verified QA Verified Feature:Autofill Address and Credit Card autofill
Milestone

Comments

@gabrielluong
Copy link
Member

gabrielluong commented Apr 22, 2022

Replace the existing country input text field with a dropdown of country values.

Examine how the Desktop Country dropdown is generated (could be this https://searchfox.org/mozilla-central/source/toolkit/components/formautofill/addressmetadata/addressReferences.js but should verify). We're assuming we want to implement a similar Country dropdown instead of having users manually enter their country, but we can also limit this to only US/CA since we are only supporting those countries initially. Ideally, we should see if Android provides something for us to validate addresses or provides a list of countries as well.

Acceptance Criteria

  1. Ensure we are displaying the same exact Country dropdown list as desktop and the expected Country values that AS accepts in their storage (checking for whether or not it's the short code such as US, case sensitivity?)
  2. We should autofill the user's country based on their region when they navigate to the address editor.
  3. If we have to load a json such as https://searchfox.org/mozilla-central/source/toolkit/components/formautofill/addressmetadata/addressReferences.js which contains country data and various validation metadata, we should probably file a new ticket and possibly do this in support-utils in AC. Please confer findings before proceeding.

No design was available at time of writing.

┆Issue is synchronized with this Jira Task

@gabrielluong gabrielluong added the Feature:Autofill Address and Credit Card autofill label Apr 22, 2022
@github-actions github-actions bot added the needs:triage Issue needs triage label Apr 22, 2022
@gabrielluong gabrielluong removed the needs:triage Issue needs triage label Apr 22, 2022
@MatthewTighe MatthewTighe self-assigned this May 5, 2022
@MatthewTighe
Copy link
Contributor

It looks like A-S expects the country to be in the ISO 3166 format, as seen in their SQL scheme definition.

The complete list of these can be retrieved easily on Android using Locale.getISOCountries.

It looks like the JSON referenced above has the added benefit of including format information, including what fields are required to consider an address valid for each locale.

I'd suggest that we take the simple path for this issue, and just use Locale.getISOCountries to generate a list with a dropdown. If we plan to implement the kind of validation being done by the JSON above, it might be best to try and get that validation handled at the A-S or GV layer so that the code can be shared.

@gabrielluong thoughts? Let me know if the above plan sounds actionable and I'll get started.

@gabrielluong
Copy link
Member Author

I am okay with starting with Locale.getISOCountries, but with a lot of caution. Regardless of the format, I think we have to pay a very close attention to the actual values in the 2 lists. The current consumer of the AS Addresses is Desktop. So, when I think about syncing and de-duplication, we need to be 100% sure that the Country/State values are exactly the same as Desktop since those values are presented as dropdown menu so they are immutable (not user entered). For example, if Quebec (not actually a country) was presented as "Quebec" in Desktop, but as "Québec" in Locale.getISOCountries, then we would have a big problem because it's not 1:1 matching what Desktop has hardcoded. Casing also matters, it can't be "QUEBEC" for instance.

I don't expect A-S to handle the validation for us. Their philosophy has always been to provide the storage and syncing capabilities and let the consumers do the rest. My ideal solution would be to handle the sharing and exposing of this data through GV APIs since the source of truth in this case is in this addressReferences.js.

Alternatively, the most basic solution with 100% confidence is to just provide a Country dropdown that displays only Canada and United States (matching the casing and spelling as provided in addressReferences.js) since we're only planning to support those 2 regions at the moment.

@MatthewTighe
Copy link
Contributor

Was this ticket intended to also capture a state/province dropdown? If that's the case, we might be best off by just moving forward on exposing a GV API. Switching between countries on desktop not only updates the state/province list, but changes field names and which fields are available.

In terms of just country data, I think I can say pretty definitely that desktop expects the country property of an address to be a country code. You can see here during normalization that they convert a country to uppercase, and then use it a few lines below to lookup a localized long-form country name for display. This is also done when fields are computed which happens during saving/loading of an address record.

Additionally, the country code is what's used to index entries in addressReferences.js.

Since our designs seem to show country codes (except USA should be US), I don't think we need to worry about spelling differences for countries specifically. States/provinces will be another matter entirely, as on desktop I can only find abbreviations used for US states. Those might require a design update for a longer or even multi-line dropdown.

The other design consideration is whether we'll be showing the long-form country name when the dropdown is active. It may make it easier for users to find the country they're looking for. I don't think this would impact the way we save/sync data though.

Ultimately, I think we could pretty easily implement something for US/CA and include the correct state/province data somewhat manually. We're likely to be better off by just sharing the dataset in addressReference.js through a GV API, though. Otherwise I'd worry about potential sync errors requiring migrations down the line.

I'll defer to you whether we should punt on this until we can get an appropriate API in place.

@gabrielluong
Copy link
Member Author

Was this ticket intended to also capture a state/province dropdown? If that's the case, we might be best off by just moving forward on exposing a GV API. Switching between countries on desktop not only updates the state/province list, but changes field names and which fields are available.

I put a higher estimate on this originally thinking we would do something to load a selection of the json fields from addressReferences.js as part of our API to display Country and State because I imagined we want to rename State (US) <-> Province (Canada) fields depending on the Country. Otherwise, we do have a separate ticket in the backlog for displaying the State dropdown so that doesn't need to apply in this issue.

I think we mentioned our preferences for having a GV API in one of our backlog glooming meeting, but given GV's 102 sprint backlog, I don't see any room where they would be able to provide us an API that wraps addressReferences.js any time soon. You should also file something with GV and raise it with their team. This might also be something you can go into GV and implement yourself with some mentorship by someone on their team. It won't be easy, but it could be worthwhile from a learning experience and wildly beneficial for your time here. So, I would strongly suggest considering it, but also make sure the necessary parties are notified if you chose to do it.

Since our designs seem to show country codes (except USA should be US), I don't think we need to worry about spelling differences for countries specifically. States/provinces will be another matter entirely, as on desktop I can only find abbreviations used for US states. Those might require a design update for a longer or even multi-line dropdown.

The other design consideration is whether we'll be showing the long-form country name when the dropdown is active. It may make it easier for users to find the country they're looking for. I don't think this would impact the way we save/sync data though.

I wouldn't worry about the country codes being different in the design. I would mostly likely defer to what the display name for Desktop is. I don't know what to think about getting a region's displayed name right now since that goes pretty deep into the platform https://searchfox.org/mozilla-central/search?q=getRegionDisplayNames&path=.

My first order of priority is ensuring the underlying synced values are the same for Country and State/Province. Getting the correct displayed name for Country should be easy enough for this issue, and we can think about the display name for State/Provide in the separate issue. I would be okay with just displaying the State/Province Code in the interim if that is the only thing that is provided to us in the dataset in addressReference.js. Otherwise, I would manually take what is provided in that dataset.

Ultimately, I think we could pretty easily implement something for US/CA and include the correct state/province data somewhat manually. We're likely to be better off by just sharing the dataset in addressReference.js through a GV API, though. Otherwise I'd worry about potential sync errors requiring migrations down the line.

I'll defer to you whether we should punt on this until we can get an appropriate API in place.

So, I would suggest for the time being to move forward with an interim solution that would be suitable for our needs. The most likely candidate would be just take the data in addressReferences.js and arrange it as a static list manually since we want to ensure the data is synced 1:1 to whatever values are on desktop.

MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue May 13, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue May 13, 2022
@MatthewTighe
Copy link
Contributor

I talked with Agi a bit yesterday about the best way to share the address reference data between desktop and mobile. He and I agree that any API to retrieve it at runtime through GV -> gecko would be fairly awkward. He suggested that we move address reference into a JSON file and generate static classes for desktop and mobile for it during central builds. With that method we can create an idiomatic class in Java and host it in GV and recreate the initial JS file for desktop.

I think that's likely out-of-scope for this ticket. Would it be best to file a follow-up against GV in bugzilla?

@gabrielluong
Copy link
Member Author

gabrielluong commented May 14, 2022

I talked with Agi a bit yesterday about the best way to share the address reference data between desktop and mobile. He and I agree that any API to retrieve it at runtime through GV -> gecko would be fairly awkward. He suggested that we move address reference into a JSON file and generate static classes for desktop and mobile for it during central builds. With that method we can create an idiomatic class in Java and host it in GV and recreate the initial JS file for desktop.

I think that's likely out-of-scope for this ticket. Would it be best to file a follow-up against GV in bugzilla?

This makes sense. One of my early thoughts was to move objects in the jsm into a json and just consume it. I would file GV issues through in Bugzilla and not track it here.

MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue May 16, 2022
@MatthewTighe
Copy link
Contributor

bugzilla ticket to expose address data

@sheikh-azharuddin
Copy link

Why only 2 countries visible in the list? US and canada

@delia-pop
Copy link

Verified as fixed on Nightly 102 from 05/25.
CountryDropdownList_24917

@delia-pop delia-pop added the eng:qa:verified QA Verified label May 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:qa:verified QA Verified Feature:Autofill Address and Credit Card autofill
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants