Skip to content

fix(intl-phone-input): remove changing unclearable country code #730

Merged
merged 7 commits into from
Jul 19, 2021

Conversation

koretskyhub
Copy link
Contributor

ПРОДОЛЖАЕТ #703

Опишите проблему

Если каретка стоит до или внутри кода страны, пользователь начинает вводить символы, то каретка перемещается в конец кода страны

Шаги для воспроизведения

Ожидаемое поведение

Если каретка стоит до или внутри кода страны и пользователь начинает вводить символы, то символы должны добавляться после кода страны

Чек лист

  • Тесты
  • Документация

Внешний вид

Ожидаемый Фактический
** Ожидаемый ** ** Фактический **

Тестовый стенд

Десктоп (если данных нет оставте блок пустым):

Смартфон (если данных нет оставте блок пустым):

Дополнительная информация

Дополнительная информация

@alfa-bot
Copy link
Collaborator

alfa-bot commented Jul 1, 2021

Собрана новая демка.

@reme3d2y
Copy link
Contributor

reme3d2y commented Jul 2, 2021

Я чет не увидел разницы. Плюс каретка после введенного символа прыгает в конец инпута.

Будет круто, если сделаешь видосы\гифки до\после

@koretskyhub
Copy link
Contributor Author

@reme3d2y, Конечно

Шаги для воспроизведения

перейти на страницу оплаты мобильной связи
кликнуть в поле ввода номера телефона
ввести цифру
кликнуть перед +7 (или между + и 7)
ввести цифру
Ожидаемый результат: цифра встанет между +7 и введенной цифрой ранее

Фактический результат: цифра заменяет +7 или встает между + и 7

было

стало

@koretskyhub
Copy link
Contributor Author

koretskyhub commented Jul 6, 2021

Эта проблема актуальна, когда проп clearableCountryCode - false

2021-07-06.10.48.15.mov

Copy link
Contributor

@reme3d2y reme3d2y left a comment

Choose a reason for hiding this comment

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

Вот такой кейс нашел

https://monosnap.com/file/1VX1vH61L1ZG7WqQFGWV6u6aU9F9cD

@koretskyhub koretskyhub requested a review from reme3d2y July 7, 2021 10:59
@alfa-bot
Copy link
Collaborator

alfa-bot commented Jul 7, 2021

Собрана новая демка.

@koretskyhub
Copy link
Contributor Author

@reme3d2y fixed

@reme3d2y
Copy link
Contributor

reme3d2y commented Jul 8, 2021

@koretskyhub
Раз уж ты погрузился в компонент — глянь плз сразу кейс, когда номер вставляется через 8 (например 89990312121). В обычном PhoneInput все ок, а тут нет

@AleksandrSerov
Copy link
Contributor

Эта проблема актуальна, когда проп clearableCountryCode - false

2021-07-06.10.48.15.mov

Хорошо бы, если бы каретка вставала не в конец инпута, а после введенного символа

@koretskyhub
Copy link
Contributor Author

@reme3d2y @AleksandrSerov done done

@alfa-bot
Copy link
Collaborator

alfa-bot commented Jul 9, 2021

Собрана новая демка.

import { getPhoneDiff } from './get-phone-diff';

const RUSSIAN_DIAL_CODE = '7';
const RUSSIAN_ALTERNATIVE_DIAL_CODE = '8';
Copy link
Contributor

Choose a reason for hiding this comment

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

В контексте libphonenumber-js это называется RUSSIAN_NATIONAL_DIAL_CODE

@alfa-bot
Copy link
Collaborator

Собрана новая демка.

}
}

input.addEventListener('keyup', resetCaret);
Copy link
Contributor

Choose a reason for hiding this comment

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

Обработчики тогда ведь при каждом ререндере будут создаваться?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

да похоже

@AleksandrSerov
Copy link
Contributor

AleksandrSerov commented Jul 13, 2021

Это очень странно выглядит конечно с прыгающей кареткой
https://user-images.githubusercontent.com/33555131/125448434-0fea38fd-5f9d-4eeb-bdc4-a86558c77da9.mov
Что дизайнеры думают?

@koretskyhub
Copy link
Contributor Author

@AleksandrSerov, я убрал свдиг каретки на код страны по кнопке.
Поправить то же для мышкой кажется никак не выйдет. Слава в курсе, все окнул

@alfa-bot
Copy link
Collaborator

Собрана новая демка.

@alfa-bot
Copy link
Collaborator

Собрана новая демка.

Copy link
Contributor

@reme3d2y reme3d2y left a comment

Choose a reason for hiding this comment

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

Думаю этот ПР можно уже принять.

Нам мой взгляд компонент еще стоит допилить, но это можно сделать отдельно и позже.

  1. Не очень нравится, что блокируется каретка
  2. Не нравится, что даже если страна уже определилась, все равно можно ввести лишние символы. К тому же в таком случае слетает форматирование.
  3. В PhoneInput между цифрами дефисы. Стоит сделать консистентно везде

if (!input) return;

const selectionStart = input.selectionStart || 0;
const toLeftKey = event.keyCode === 37;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: можно на event.key заменить

@koretskyhub
Copy link
Contributor Author

Думаю этот ПР можно уже принять.

Нам мой взгляд компонент еще стоит допилить, но это можно сделать отдельно и позже.

  1. Не очень нравится, что блокируется каретка
  2. Не нравится, что даже если страна уже определилась, все равно можно ввести лишние символы. К тому же в таком случае слетает форматирование.
  3. В PhoneInput между цифрами дефисы. Стоит сделать консистентно везде

Есть тасочка с еще одним кейсом для этого компонента, закину туда эти кейсы

@reme3d2y reme3d2y merged commit 6d219d6 into master Jul 19, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/intl-phone-input branch July 19, 2021 15:02
dmitrsavk pushed a commit that referenced this pull request Jul 19, 2021
# [19.2.0](v19.1.0...v19.2.0) (2021-07-19)

### Bug Fixes

* **intl-phone-input:** remove changing unclearable country code ([#730](#730)) ([6d219d6](6d219d6))

### Features

* **phone-input:** add 'clearableCountyCode' prop ([#749](#749)) ([d110ae7](d110ae7))
* **select:** add scroll handler ([#756](#756)) ([b25351b](b25351b))
* **tooltip:** add target ref property ([#755](#755)) ([9aa962d](9aa962d))
* **typography:** monospaceNumbers prop (PDS-255) ([#697](#697)) ([42d16a6](42d16a6))
@dmitrsavk
Copy link
Contributor

🎉 This PR is included in version 19.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants