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

feat(components): add new component #40

Merged

Conversation

zorox112-practicum
Copy link
Collaborator

No description provided.

@@ -0,0 +1,47 @@
import React from 'react';

const getDiffDays = (date: Date) =>
Copy link

Choose a reason for hiding this comment

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

Давай утилки вынесем в отдельный файлик и подберем понятные имена (getFormattedUnit)

Copy link

Choose a reason for hiding this comment

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

еще можно взглянуть на либу https://www.npmjs.com/package/pluraljs и похожие, тк есть подозрение, что такое понадобится

Copy link
Collaborator Author

@zorox112-practicum zorox112-practicum Sep 13, 2022

Choose a reason for hiding this comment

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

@ArchieSA

Давай утилки вынесем в отдельный файлик и подберем понятные имена (getFormattedUnit)

Да согласен

еще можно взглянуть на либу...

Тут не очень понял на самом деле зачем либа, текущий код работает, а наша либа (react-developer-burger-ui-components) щас так красиво выглядит с минимумом зависимостей (не дев зависимостей), что не хочу ничего тащить если честно, а особенно вот такие конфиги:
https://github.com/kalpeshsingh/PluralJS/blob/master/src/helpers/dictionary.js#L5

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Так-с, в итоге нашел достаточно легкую либу plural-ru :)

Утилитки перенес, но не уверен по структуре, я не нашел нигде не плоскую структуру для компонента, но кидать утилитки на уровень папки UI компонентов чет не хочется, если честно и по неймингу файлов тоже не гарантирую (возможно тут используется какая-то общепринятая структура, о которой я не знаю буду признателен, если ткнешь пальцем, где о ней почитать)

@yandex-praktikum
Copy link
Collaborator

Коллеги, призовите меня пожалуйста как PR будет готов к вливанию и пройдет код-ревью

@zorox112-practicum zorox112-practicum force-pushed the PCCWF-371 branch 3 times, most recently from 39eae69 to 6474e59 Compare October 10, 2022 19:35
@ArchieSA
Copy link

OK

@zorox112-practicum
Copy link
Collaborator Author

@yandex-praktikum

Код-ревью прошел, так что можно вливать

@zorox112-practicum
Copy link
Collaborator Author

@yandex-praktikum

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

@zorox112-practicum
Copy link
Collaborator Author

@yandex-praktikum

Готово, пофиксил registry, поребейсился, локально все огонь

@zorox112-practicum zorox112-practicum merged commit 829a5c7 into Yandex-Practicum:main Nov 14, 2022
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.

3 participants