Conversation
There was a problem hiding this comment.
для работы с svg очень популярна вот эта тема https://react-svgr.com, инлайнить их плохая практика
Оказывается эта тема поддерживается с версии 2.0.0, а у нас 1.1.1((
| props.articles.map(article => { | ||
| return ( | ||
| <ArticlePreview article={article} key={article.slug} /> | ||
| ); | ||
| }) |
There was a problem hiding this comment.
можно в одну строчку
map(article => <ArticlePreview ... />);
There was a problem hiding this comment.
Мне кажется, что в таком виде код более читаемый. Разве нет?
There was a problem hiding this comment.
к такой короткой записи легко привыкнуть, это нормально
There was a problem hiding this comment.
встречается все чаще, действительно быстро привыкаешь, ну и не зря же все эти упрощения вводятся
| ); | ||
| } | ||
|
|
||
| if (props.articles.length === 0) { |
There was a problem hiding this comment.
Ноль сам по себе дает false, так что можно просто if (props.articles.length)
There was a problem hiding this comment.
Можно и так, но тогда if (!props.articles.length)
| currentPage: undefined | ||
| }; | ||
|
|
||
| export default ArticleList; |
There was a problem hiding this comment.
Мне нравится пропсы деструктуризировать, так код лучше читается, а то от props в глазах рябит
| }); | ||
|
|
||
| const ArticlePreview = props => { | ||
| const article = props.article; |
There was a problem hiding this comment.
Я даже присмотрелся и не понял, зачем вообще такая конструкция использовалась, если уж можно деструктуризацию сделать вообще используемых пропсов.
Видимо, когда переписывал просто записал так, чтобы можно было пропс прокинуть дальше
There was a problem hiding this comment.
это просто деструктуризация, считай современный синтаксис.
А так да, если надо прокинуть все пропсы, обычно так делают
| <div className="strange-block" /> | ||
| <div className="article-preview"> |
There was a problem hiding this comment.
тут же должны быть <StrangeBlock /> и <ArticlePreviewBlock />
There was a problem hiding this comment.
А смысл, если стили к этим дивам можно прописать в компоненте ArticleWrapper и не городить лишних структур? Или есть какие-то рекомендуемые практики?
There was a problem hiding this comment.
Как по мне, подход styled component предполагает использование именно компонентов для стилизации. Классы как-то выбиваются из этой концепции. Сам я первый раз использую этот подход, но смесь компонентов и классов не приходилось встречать
There was a problem hiding this comment.
Но для каких целей тогда имеется возможность стилизовать дочерние блоки?
There was a problem hiding this comment.
Если бы я знал) Посмотрим, что Влад ответит.
Спросил пока товарища по опытнее, любителя этой технологии, говорит "Класснеймы примешивать моветон, как мне кажется. Работаешь с компонентами - работай с компонентами". Но опять же "как мне кажется"
| if ( src === defaultAvatarURL ) { | ||
| src = defaultAvatarPath; | ||
| } | ||
| const isDefault = (src === defaultAvatarPath); |
There was a problem hiding this comment.
чтобы два раза не вызывать сравнение, можно так:
`let isDefault = false;
if ( src === defaultAvatarURL ) {
src = defaultAvatarPath;
isDefault = true;
}`
| ${props => { | ||
| switch (props.location) { | ||
| default: | ||
| case 'profile': { | ||
| return 'width: 120px; height: 120px; margin-bottom: 8px;'; | ||
| } | ||
| case 'article': { | ||
| return 'width: 48px; height: 48px; margin-right: 8px;'; | ||
| } |
There was a problem hiding this comment.
switch case кажется избыточным, так всего два кейса. Их можно обработать просто есть пропс/нет пропса. И default и big/large кажется более подходящим, вдруг еще где придется потом их использовать
There was a problem hiding this comment.
А вдруг нужен будет другой размер для другого места?
There was a problem hiding this comment.
Тогда все гуд. Написал коммент исходя из того, что у нас дизайн 100% не поменяется и кейса два
| & .author { | ||
| color: #0A0A0B; | ||
| font-size: 16px; | ||
| line-height: 24px; | ||
| } | ||
|
|
||
| & .date { | ||
| color: #62626A; | ||
| font-size: 12px; | ||
| line-height: 16px; | ||
| } |
| <Link | ||
| className="nav-link " | ||
| to={`/`} | ||
| onClick={clickHandler} | ||
| > |
| ); | ||
| } else { | ||
| return ( | ||
| <ul className="nav nav-pills outline-active"> |
3 participants
No description provided.