Skip to content

fix(money-input): caret position in integer part #1096

Merged
merged 11 commits into from
Apr 17, 2020
Merged

Conversation

AleksandrSerov
Copy link
Contributor

Мотивация и контекст

Решает issue #862

@coveralls
Copy link

coveralls commented Mar 4, 2020

Coverage Status

Coverage increased (+0.05%) to 85.205% when pulling 68de0a4 on fix/money-input into d603098 on master.

/**
* Устанавливает каретку поля ввода в новую позицию.
*
* @param {Number} selection Новое положение каретки
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Может лучше описать на TypeScript:

    private setInputSelection(selection: number) {

Ну и тогда из документации можно удалить {Number}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Описал

@@ -106,6 +111,8 @@ export class MoneyInput extends React.PureComponent<MoneyInputProps, MoneyInputS

root: HTMLInputElement;

private caretFixTimeout: ReturnType<typeof setTimeout> = null;;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private caretFixTimeout: ReturnType<typeof setTimeout> = null;;
private caretFixTimeout: ReturnType<typeof setTimeout> = null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Поправил

@SiebenSieben
Copy link
Contributor

SiebenSieben commented Mar 6, 2020

  1. Ты продублировал часть masked-input в money-input? Почему так?
  2. Мы дропаем IE10, можно эту часть убрать.

@AleksandrSerov
Copy link
Contributor Author

AleksandrSerov commented Mar 6, 2020

  1. Ты продублировал часть masked-input в money-input? Почему так?
  2. Мы дропаем IE10, можно эту часть убрать.

Меньше зависимости от masked-input.
Можно сделать setInputSelection в masked-input публичным. Будет меньше дублирования

README.md Outdated
@@ -73,11 +73,11 @@ npm install arui-feather --save
Шаги запуска демо:

1. `npm i`
2. `npm run demo`
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.

@stepancar Поправлено

// IE10 не умеет синхронно в событие `change` переставлять каретку.
// Android chrome имеет дефект с автоматической установкой каретки
// при использовании клавиатуры отличной от type="text".
if (IS_IE9_10 || IS_ANDROID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@SiebenSieben IE11 у нас же уже

componentWillUnmount() {
if (this.caretFixTimeout) {
clearTimeout(this.caretFixTimeout);
this.caretFixTimeout = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Для чего эта проверка и установка прям перед дестроем этого флага в null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Переносил часть кода из masked-input.
Упростил

@@ -20,6 +20,11 @@ const DEFAULT_FRACTION_SIZE = 2;
const DEFAULT_INTEGER_SIZE = 9;
const INTEGER_PART_SIZE = 3;

// В эту проверку попадают IE9 и IE10, которые не могут корректно работать с кареткой на событии `input`.
const IS_IE9_10 = typeof window !== 'undefined' && !!(window as any).ActiveXObject;
Copy link
Contributor

Choose a reason for hiding this comment

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

Это надо убрать. IE 11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Убрал

if (newValue.length > currentValue.length && newValue[currentSelection] === ',') {
setTimeout((() => {

// Фикс бага смещения каретки в браузере на андроидах Jelly Bean (c 4.1 по 4.3)
Copy link
Contributor

Choose a reason for hiding this comment

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

мы поддерживаем такие старые версии?
это вроде как устаревшие версии, которые были актулальны в 2013 (7 лет назад).
Есть какая-то статистика, в которой отображено сколько людей к нам заходит со старыми версиями андроида?

@SiebenSieben SiebenSieben merged commit fdf0d87 into master Apr 17, 2020
@SiebenSieben SiebenSieben deleted the fix/money-input branch April 17, 2020 08:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Некорректно вводятся числа компонента money_input
7 participants