search

feature-sliced / documentation

🍰 Architectural methodology for frontend projects
https://feature-sliced.design
MIT License
1.41k stars 146 forks source link

Add React/Redux example #612

Closed Nyar1othotep closed 8 months ago

Nyar1othotep commented 10 months ago

Background

Привет! Хотел бы добавить и свой простой сайт.

  1. Live Demo
  2. Repo

Буду рад code review.

netlify[bot] commented 10 months ago

Deploy request for pr-fsd accepted.

Name Link
Latest commit cc5acdbd5212fe3c3011e379822c4852ec26b2e6
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/64df68d13d2eec0008abc559
Nyar1othotep commented 10 months ago

Почитал ревью на pull request`ы других разработчиков, начну исправлять свои недочеты и еще раз прочитаю статью. Пока перейду в draft.

Nyar1othotep commented 9 months ago

Извеняюсь, что так долго. Исправил по мелочи. Буду рад код рьею. Заранее спасибо.

Nyar1othotep commented 9 months ago

@azinit

azinit commented 9 months ago

@feature-sliced/core

illright commented 9 months ago
  1. shared/lib/hooks -- hooks -- довольно неописательное название, оно говорит нам, что в папке хуки, но не говорит нам, для чего эти хуки. Хуки бывают очень разными, от UI до API, обычно лучше группировать их именно по назначению
  2. pagination -- так себе сущность в контексте приложения галерии картин. В этом приложении я бы выделил только одну сущность -- картину, а все остальное -- это элементы интерфейса. Возможность менять тему -- это к shared или features, пагинация -- скорее к виджетам, фильтры -- фича.
  3. Заметил, что у тебя фичи такие же, как и сущности. Это не очень хорошо, потому что семантически, сущности и фичи -- совершенно разные вещи, как видно, например, на памятке по декомпозиции.

К остальному вопросов нет. Из списка выше я бы выделил блокерами 2 и 3, 1 -- на твое усмотрение. Напиши, что думаешь насчет этого фидбека

Nyar1othotep commented 8 months ago

Спасибо за фидбек.

  1. shared/lib/hooks -- hooks -- довольно неописательное название, оно говорит нам, что в папке хуки, но не говорит нам, для чего эти хуки. Хуки бывают очень разными, от UI до API, обычно лучше группировать их именно по назначению

Так как у меня в hooks хуки для изображения, то мне следуют сделать что-то вроде: src/lib/image/, где и будут хранится хуки и вспомогательные функции по назначению относящиеся к изображению. Если я верно понял.

  1. pagination -- так себе сущность в контексте приложения галерии картин. В этом приложении я бы выделил только одну сущность -- картину, а все остальное -- это элементы интерфейса. Возможность менять тему -- это к shared или features, пагинация -- скорее к виджетам, фильтры -- фича. Спасибо за фидбек.

  2. Фильтры: действительно фильтры стоит относить к фиче.

  3. pagination: хотелось бы отнести пагинацию к фиче, но в целом у меня есть представление как я могу отнести к виджету.

  4. Тема: как я правильно понял, theme из сущности, стоит перенести в фичу. Посмотрев памятку, всё немного проясняется для меня.

  5. Заметил, что у тебя фичи такие же, как и сущности. Это не очень хорошо, потому что семантически, сущности и фичи -- совершенно разные вещи, как видно, например, на памятке по декомпозиции.

Как и говорил, после памятки, всё немного проясняется для меня. Этот пункт я так же исправлю.

Пока перейду в draft на исправление своих ошибок. Еще раз большое спасибо.

netlify[bot] commented 8 months ago

Deploy Preview for pr-fsd ready!

Name Link
Latest commit 9f4c7d748947e595baff41e8eef8ca8a8762a3ca
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/6529418d11178f00079235a5
Deploy Preview https://deploy-preview-612--pr-fsd.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Nyar1othotep commented 8 months ago

@illright , Привет. Я исправил сайт. Буду рад еще одной проверке. Отпиши, если все теперь хорошо, пожалуйста. После этого я закрою этот Pull Request, так как считаю, что код моего сайта явно не пример для других. Я еще только новичок и мне есть куда расти.

illright commented 8 months ago

Да, теперь все хорошо. Из дополнительных рекомендаций – хочется навести порядок в shared/lib, это такое довольно абстрактное место, за которым нужно тщательно следить, чтоб оно в процессе развития проекта не превратилось в помойку для всего подряд. Например, вижу там хук usePageCount, который используется только виджетом ArtworksNavigation, можно сложить этот хук в сегмент lib в слайсе этого виджета (widgets/artworks).

Насчет того, что тебе кажется, что твой проект — не пример для других: я считаю, что многие проекты могут чему-то научить, твой в частности. Это правильно построенный по FSD проект, все-таки, поэтому мне кажется, что он достоин того, чтоб быть в примерах, но не настаиваю.

Nyar1othotep commented 8 months ago

Да, теперь все хорошо. Из дополнительных рекомендаций – хочется навести порядок в shared/lib, это такое довольно абстрактное место, за которым нужно тщательно следить, чтоб оно в процессе развития проекта не превратилось в помойку для всего подряд. Например, вижу там хук usePageCount, который используется только виджетом ArtworksNavigation, можно сложить этот хук в сегмент lib в слайсе этого виджета (widgets/artworks).

Насчет того, что тебе кажется, что твой проект — не пример для других: я считаю, что многие проекты могут чему-то научить, твой в частности. Это правильно построенный по FSD проект, все-таки, поэтому мне кажется, что он достоин того, чтоб быть в примерах, но не настаиваю.

Да, с shared/lib у меня конечно возникли трудности. В целом, спасибо за хорошие слова. Благодаря этим фидбекам, я еще больше аспектов для себя подчеркнул. В будущем, постараюсь сделать проект намного лучше и обязательно приду сюда еще раз. У меня все еще есть желание внести вклад в FSD, пусть и просто примером.