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

Feature/payment widget #123

Merged
merged 51 commits into from
Aug 25, 2023
Merged

Feature/payment widget #123

merged 51 commits into from
Aug 25, 2023

Conversation

KentCpu
Copy link
Contributor

@KentCpu KentCpu commented Jul 31, 2023

После долгих раздумий решил погуглить. Нашел схожий функционал у "ЮKassa". Они предоставляют конструктор формы с обязательными полями и возможностью добавления дополнительных. Я попытался реализовать нечто подобное в коде. Что касается стилей, я заметил, что они обычно фиксированы для таких форм. Смог получить доступ к функции "pay", но мне кажется, что это костыль.

Прошу ответить на следующие вопросы:

  1. Правильно ли я понимаю, что стили обычно фиксированы для таких форм, или следует предоставить возможность для их стилизации?
  2. Как можно было получить доступ к функции pay по другому ? Смог сделать только так

@KentCpu KentCpu added the enhancement New feature or request label Jul 31, 2023
@KentCpu KentCpu self-assigned this Jul 31, 2023
@TorinAsakura
Copy link
Member

После долгих раздумий решил погуглить. Нашел схожий функционал у "ЮKassa". Они предоставляют конструктор формы с обязательными полями и возможностью добавления дополнительных. Я попытался реализовать нечто подобное в коде. Что касается стилей, я заметил, что они обычно фиксированы для таких форм. Смог получить доступ к функции "pay", но мне кажется, что это костыль.

Прошу ответить на следующие вопросы:

  1. Правильно ли я понимаю, что стили обычно фиксированы для таких форм, или следует предоставить возможность для их стилизации?
  2. Как можно было получить доступ к функции pay по другому ? Смог сделать только так
  1. Для дискуссий - есть дискусси.
  2. Мы пишем библиотеку, а это значит, что платёжка должна поддаваться лёгкой кастомизации. Ярым моветоном считается хардкод в любом проявлении. От того, что ты потратишь лишние полчаса и вынесешь стили в отдельный слой ничего плохого не случится.
  3. Понятия не имею, чтобы разобраться в данном вопросе - нужно разобраться с самим плагином. Но, если говорить про архитектуру, то это должно выглядеть так:
  • инициализация
  • публикация функции
  • расширение по месту использования
  1. При этом, фичу нужно разбивать на слои, дабы не создавать монолитный кусок кода, как это сделано у тебя сейчас

Copy link
Member

@TorinAsakura TorinAsakura left a comment

Choose a reason for hiding this comment

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

  • Обрати внимание на отсутствие последних строк
  • использований нативных хтмл компонентов - запрещено, как ты потом будешь с ним работать? через some.ref[div]? Используй нашу библиотеку github.com/atls/hyperion
  • все сообщения, должны быть в формате intl, пример есть в github.com/torin-asakura/drumin, ищи FormattedMessage
  • далее, у нас отступы 2, а у тебя 4, код выглядит раздутым, используй пожалуйста yarn check перед тем как коммитить
  • пожалуйста, используй систему монорепозиториев, посмотри как выполнены плагины рядом
  • в packages/payment-widget/src/payment-widget.hook.tsx намешано всё подряд, так делать не нужно, разбивай на ответственные зоны: change.handler.ts, init.hook.ts (туда можешь вынести инициализацию document.createElement)
  • всевозможные ошибки должны быть вынесены в слой exception, пример https://github.com/atls/nestjs/blob/cf76057580570b056a033569ff37ac7bac9f0070/packages/grpc-errors/src/exception-factories/grpc-validation.exception-factory.ts#L23

@KentCpu KentCpu requested a review from Nelfimov August 4, 2023 16:26
Copy link
Member

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

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

Необходимо доработать.

packages/payment-widget/package.json Outdated Show resolved Hide resolved
packages/payment-widget/package.json Outdated Show resolved Hide resolved
packages/payment-widget/package.json Outdated Show resolved Hide resolved
packages/payment-widget/src/utils/init.util.ts Outdated Show resolved Hide resolved
packages/payment-widget/src/hooks/use-fields.hook.tsx Outdated Show resolved Hide resolved
@KentCpu KentCpu requested a review from Nelfimov August 9, 2023 12:48
Copy link
Member

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

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

Хороший прогресс, работаем дальше.

packages/payment-widget/package.json Outdated Show resolved Hide resolved
packages/payment-widget/src/utils/convertToKopecks.util.ts Outdated Show resolved Hide resolved
packages/payment-widget/src/utils/mergeFields.util.ts Outdated Show resolved Hide resolved
packages/payment-widget/src/hooks/use-fields.hook.tsx Outdated Show resolved Hide resolved
packages/payment-widget/src/hooks/use-fields.hook.tsx Outdated Show resolved Hide resolved
packages/payment-widget/src/data/fields.data.ts Outdated Show resolved Hide resolved
packages/payment-widget/src/index.ts Outdated Show resolved Hide resolved
packages/payment-widget/src/hooks/use-fields.hook.tsx Outdated Show resolved Hide resolved
@KentCpu
Copy link
Contributor Author

KentCpu commented Aug 18, 2023

@Nelfimov, я решил отказаться от идеи с темой для кнопки, так как это выглядело не очень органично. Вместо этого, я использовал стили из формы для оплаты и добавил валидацию. Жду ревью :)

@KentCpu KentCpu requested a review from Nelfimov August 21, 2023 13:29
Copy link
Member

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

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

👍

@KentCpu KentCpu requested a review from Nelfimov August 25, 2023 08:36
@TorinAsakura TorinAsakura merged commit cc50457 into master Aug 25, 2023
6 checks passed
@TorinAsakura TorinAsakura deleted the feature/payment-widget branch August 25, 2023 16:53
@TorinAsakura TorinAsakura linked an issue Aug 30, 2023 that may be closed by this pull request
@TorinAsakura TorinAsakura mentioned this pull request Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tinkoff Widget
3 participants