Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/components/phone_input_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
<%= content_tag(
'lg-phone-input',
class: tag_options[:class],
data: { delivery_methods: delivery_methods },
data: {
delivery_methods: delivery_methods,
translated_country_code_names: translated_country_code_names,
},
) do %>
<%= content_tag(
:script,
Expand Down
18 changes: 14 additions & 4 deletions app/components/phone_input_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,25 @@ def initialize(
end

def supported_country_codes
codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.keys
codes &= allowed_countries if allowed_countries
codes
@supported_country_codes ||= begin
codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.keys
codes &= allowed_countries if allowed_countries
codes
end
end

def translated_country_code_names
supported_country_codes.map do |code|
code = code.downcase
[code, I18n.t("countries.#{code}")]
end.to_h
end

def international_phone_codes
supported_country_codes.
map do |code_key|
code_data = PhoneNumberCapabilities::INTERNATIONAL_CODES[code_key]
code_data = PhoneNumberCapabilities.translated_international_codes[code_key]

[
international_phone_code_label(code_data),
code_key,
Expand Down
8 changes: 7 additions & 1 deletion app/javascript/packages/phone-input/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,20 @@ export class PhoneInput extends HTMLElement {
/** @type {string[]} */
deliveryMethods = [];

/** @type {Object.<string,*>} */
Copy link
Copy Markdown
Contributor

@aduth aduth May 12, 2022

Choose a reason for hiding this comment

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

It looks to me like it works in practice, though my understanding is that it's recommended to avoid the uppercase "Object" type (along with a few others). Also, would we be safe to assume that the expected value should be a string?

Maybe one of these:

Suggested change
/** @type {Object.<string,*>} */
/** @type {Record<string, string>} */
Suggested change
/** @type {Object.<string,*>} */
/** @type {object} */

The latter comes from the DefinitelyTyped types for the library, in case we want to align to that, and optionally add it as a devDependency to our project.

countryCodePairs = {};

connectedCallback() {
/** @type {HTMLInputElement?} */
this.textInput = this.querySelector('.phone-input__number');
/** @type {HTMLSelectElement?} */
this.codeInput = this.querySelector('.phone-input__international-code');
this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper');
this.exampleText = this.querySelector('.phone-input__example');

try {
this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || '');
this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || '');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not that I would expect it to fail, but one downside of grouping these assignments in the same try block is that if deliveryMethods fails to parse, we won't assign countryCodePairs, even if countryCodePairs could be parsed.

To me, it seems like either we should trust that they'd always parse correctly and avoid the try blocks altogether, or if we want to be guarded about it, we should have two separate try blocks, one for each.

Suggested change
this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || '');
} catch {}
try {
this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || '');

} catch {}

if (!this.textInput || !this.codeInput) {
Expand Down Expand Up @@ -118,11 +123,12 @@ export class PhoneInput extends HTMLElement {
}

initializeIntlTelInput() {
const { supportedCountryCodes } = this;
const { supportedCountryCodes, countryCodePairs } = this;
const allowDropdown = supportedCountryCodes && supportedCountryCodes.length > 1;

const iti = intlTelInput(this.textInput, {
preferredCountries: ['US', 'CA'],
localizedCountries: countryCodePairs,
onlyCountries: supportedCountryCodes,
autoPlaceholder: 'off',
allowDropdown,
Expand Down
15 changes: 15 additions & 0 deletions app/javascript/packages/phone-input/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,14 @@ describe('PhoneInput', () => {
isSingleOption = false,
isNonUSSingleOption = false,
deliveryMethods = ['sms', 'voice'],
translatedCountryCodeNames = {},
} = {}) {
const element = document.createElement('lg-phone-input');
element.setAttribute('data-delivery-methods', JSON.stringify(deliveryMethods));
element.setAttribute(
'data-translated-country-code-names',
JSON.stringify(translatedCountryCodeNames),
);
element.innerHTML = `
<script type="application/json" class="phone-input__strings">
{
Expand Down Expand Up @@ -165,4 +170,14 @@ describe('PhoneInput', () => {
);
});
});

context('with translated country code names', () => {
it('renders the translated label', () => {
createAndConnectElement({ translatedCountryCodeNames: { us: 'Custom USA' } });

const itiOptionName = document.querySelector('[data-country-code="us"] .iti__country-name');

expect(itiOptionName.textContent).to.equal('Custom USA');
});
});
});
40 changes: 39 additions & 1 deletion spec/components/phone_input_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
**tag_options,
}.compact
end
let(:instance) { described_class.new(**options) }

subject(:rendered) do
render_inline(described_class.new(**options))
render_inline(instance)
end

it 'renders an lg-phone-input tag' do
Expand All @@ -42,6 +43,28 @@
)
end

describe '#supported_country_codes' do
subject { instance.supported_country_codes }

it { is_expected.to start_with(['AD', 'AE', 'AF', 'AG']) }

context 'with allowed_countries' do
let(:allowed_countries) { ['US', 'CA'] }

it { is_expected.to eq(['CA', 'US']) }
end
end

describe '#translated_country_code_names' do
subject { instance.translated_country_code_names }

before do
I18n.locale = :es
end

it { is_expected.to include('us' => 'Estados Unidos') }
end

context 'with class tag option' do
let(:tag_options) { { class: 'example-class' } }

Expand Down Expand Up @@ -72,6 +95,21 @@
end
end

context 'when the locale has been changed' do
before do
I18n.locale = :es
end

let(:allowed_countries) { ['US'] }

it 'translates the allowed country name' do
expect(rendered).to have_select(
t('components.phone_input.country_code_label'),
options: ['Estados Unidos +1'],
)
end
end

context 'with sms delivery constraint' do
let(:delivery_methods) { [:sms] }

Expand Down