Skip to content
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

Phone number sharing api #49

Closed
wants to merge 9 commits into from

Conversation

ankur2136
Copy link
Contributor

  1. Implement Phone number sharing API.

These first 2 screen shots show how the dialog pops up and use can choose to select their verified phone numbers.

Screenshot 2023-10-13 at 11 08 53 AM Screenshot 2023-10-13 at 11 09 11 AM
  1. Fix the country code selector to auto-change to autofilled phone number
    Instead of filling just the PhoneNumberNation, the changes would auto-populate the country code from the filled number
    See screenshot1 and 2 for reference where original selected country was India but the autofill populated a number starting +1-858 ... which changed the country code to +1 (US)

  2. If the user dismisses the first bottomsheet, then a regular keyboard with autofill is triggered. (Screenshot3)

Screenshot 2023-10-13 at 11 13 39 AM

which also suggests an autofill number (this was the original implementation)
Selecting this number also leads to the same result on screenshot2 which autofills the phone number as well as country code.

Followup:

  1. Update Readme.
  2. Update as you type number's country code parsing.

@sonarcloud
Copy link

sonarcloud bot commented Oct 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@hbmartin
Copy link
Contributor

Thank you! 🚀 🚀 🚀 I'll take a closer look soon.
Wonder if there's a way to package this in another module to avoid needing to ship appcompat/play, I'll think about it over the weekend.

@ankur2136
Copy link
Contributor Author

Thank you! 🚀 🚀 🚀 I'll take a closer look soon. Wonder if there's a way to package this in another module to avoid needing to ship appcompat/play, I'll think about it over the weekend.

I've verified that adding the play-services-auth library is not making any difference in the lib size.

App compat addition is only to fix one of the warnings from sonar which is not a bug but a misconfigured version. (https://stackoverflow.com/a/66740530). The app works the exact same way even without adding the dependency.

It would be easier if we can merge these changes, then split the whole code into a few modules

  1. Core/data
  2. M2 support -- legacy
  3. M3 support
  4. Optionally a variant without play-services-auth -- As a developer consuming this library, I wouldn't be worried given this is optional and would be stripped out by proguard if not used.

@ankur2136
Copy link
Contributor Author

Closing since it seems no one wants to merge this.

@ankur2136 ankur2136 closed this Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants