Simplify amount implementation
Closed this issue · 6 comments
https://habr.com/ru/post/491120/
@nin-jin
Указал нам на проблемы amount
Предлагает дойти до реализации в таком вот виде
<h3 class="amount">
<span class="amount__major">1 233</span>
<span class="amount__minor">,43 ₽</span>
</h3>
Большинство нод в свое время было добавлено из за разности прозрачности отдельных элементов. Думаю щас большинство не актуально.
DIv в span (facepalm)
Давайте попробуем причесать
@nin-jin, не стесняйтесь нам создавать issue, если заметили проблему, а еще лучше PR. В любом случае, Спасибо за нелестную оценку!
@nin-jin
В статье примерчик
const FeeButton = ( { id , submit , size = 'm' , theme = 'dark' , className = '' } ) => (
<Button
onClick={ submit }
className={ "fee-button__submit " + className }
data-test-id={ id }
size={ size }
theme={ theme }
>
<Label
className="fee-button__prefix"
size={ size }
theme={ theme }
>
{ l10n( 'fee-button__prefix' ) }
</Label>
<Amount
className="fee-button__fee"
data-test-id={ id + '/fee' }
amount={ fee }
size={ size }
theme={ theme }
/>
</Button>
)
Подскажите, где такой взяли?
Нигде, разумеется, просто представил что бы мне пришлось написать, используя я ваши компоненты. Если что-то лишнее - скажите ,я поправлю статью.
@nin-jin
Да не, править в статье ничего не надо)
theme у нас проставляется один раз с помощью ThemeProvider, но так же можно и явно поставить через пропсы.
для чего вам в таком примере className - тоже не понятно.
в ожидаемом примере из статьи
<Button id="pay-submit" onClick={ submit }>
<Label id="pay-prefix">{ payPrefix }</Label>
<Amount id="pay-fee" amount={ fee }/>
</Button>
вы их и не используете. А пример выглядит намного минималистичнее.
Плюс, если дальше развивать тему в "Плохом примере" вы описываете
const FeeButton = ( { id , submit , size = 'm' , theme = 'dark' , className = '' } ) => ( ....
А в вашем примере этого нет. Суммарно поэтому ваш пример выглядит много лучше. Ну это ладно)
А вот по поводу size - вы предлагаете брать size от родителя внутри вложенного компонента?
theme у нас проставляется один раз с помощью ThemeProvider, но так же можно и явно поставить через пропсы.
То есть мой компонент надо дополнительно усложнить, чтобы можно было получать размер и из пропсов и из контекста. Иначе он не будет вписываться в вашу экосистему, где можно и так и сяк.
в ожидаемом примере из статьи вы их и не используете.
Потому, что задание их через пропсы не имеет смысла. Когда один компонент собираешь из других компонент - незачем вручную прокидывать тему во вложенные компоненты.
А в вашем примере этого нет.
Потому что там не реакт вовсе. Обычный html-шаблон.
А вот по поводу size - вы предлагаете брать size от родителя внутри вложенного компонента?
Да, это тоже контекстно зависимая штука. Если я указал size для компонента, то ожидаю, что он увеличится со всем содержимым, а не только паддинги вокруг него.
theme у нас проставляется один раз с помощью ThemeProvider, но так же можно и явно поставить через пропсы.
То есть мой компонент надо дополнительно усложнить, чтобы можно было получать размер и из пропсов и из контекста. Иначе он не будет вписываться в вашу экосистему, где можно и так и сяк.
Не совсем так в вашем случае для темы ничего делать не нужно, она сама прорастет.
(Хотел собрать вам пример в нашей песочница, да как оказалось , у нас песочница сломалась как раз недавно)
А вот по поводу size - вы предлагаете брать size от родителя внутри вложенного компонента?
Да, это тоже контекстно зависимая штука. Если я указал size для компонента, то ожидаю, что он увеличится со всем содержимым, а не только паддинги вокруг него.
Тут я согласен. По имплементации как предлагаете прокидывать size? react контекстом?
Не совсем так в вашем случае для темы ничего делать не нужно, она сама прорастет.
(Хотел собрать вам пример в нашей песочница, да как оказалось , у нас песочница сломалась как раз недавно)
Это вы исходите из того, что на ваших компонентах будут делать лишь приложения, но не другие компоненты. Представьте, каково будет пользователю моих и ваших компонент вперемешку. В одних компонентах тему можно задать через пропсы, в других нельзя.
Тут я согласен. По имплементации как предлагаете прокидывать size? react контекстом?
Ну да, контекстом. Или даже просто через css-zoom.