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

Support intl-tel-input 19 #49

Merged
merged 5 commits into from
Mar 17, 2024
Merged

Support intl-tel-input 19 #49

merged 5 commits into from
Mar 17, 2024

Conversation

tlebreton
Copy link
Contributor

Hi

This PR upgrade intl-tel-input to version 19. (Don't think we need to touch at somethink else).

Thanks for the work on this libary :)

@mpalourdio
Copy link
Owner

Thanks for this PR. Looks like the new API is not backward compatible, tests are failing :/

@tlebreton
Copy link
Contributor Author

Ok i will have a look

@tlebreton
Copy link
Contributor Author

tlebreton commented Mar 14, 2024

There is the last that fails but i don't know if it's normal or not.

For me it's a normal component because there is no translation for Sweden in i18n object and by default intl-tel-input put in english.

@@ -31,7 +31,7 @@ export class IntlTelInputComponent implements AfterViewInit {
@Output() private E164PhoneNumberChange = new EventEmitter<string | null>();
@ViewChild('intlTelInput') private _inputElement!: ElementRef;
private _phoneNumber!: string;
private _intlTelInput: any;
public _intlTelInput: any;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should stay private

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups my bad was debugging somethink and forget about it ....

@@ -7,7 +7,7 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { ComponentFixture, TestBed } from '@angular/core/testing';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch ! The linter should have warned...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe updating all packages will help

@mpalourdio
Copy link
Owner

mpalourdio commented Mar 14, 2024

Thanks, I have checkout locally the PR. The onlyLocalized @Input is not needed anymore anymore. So subsequent code and tests (the one that fails) can be safely removed IMO, because country.name does not need any regexing anymore

jackocnr/intl-tel-input@03277c4 (watch diff for data.js)

@tlebreton
Copy link
Contributor Author

I remove the test as you said :)

@mpalourdio
Copy link
Owner

I remove the test as you said :)

Thanks, But this should be removed too (and readme should be updated because this option is removed) :

@mpalourdio
Copy link
Owner

Good job. Will review and polish ASAP, before the end of the week in any case. Will need to squash your commits. I hadn't seen you had forked and committed to master, so will have to do this locally, because I think i won't be able to push/force push to your master :/ (not a big deal anyway)

Thanks a lot for this contribution!

@mpalourdio mpalourdio merged commit f5f4f8a into mpalourdio:master Mar 17, 2024
3 checks passed
@mpalourdio
Copy link
Owner

v0.11.0 has been released. Thanks for contributing!

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