Lesson 4 #9
Lesson 4 #9
Conversation
.storybook/addons.js
Outdated
| @@ -0,0 +1 @@ | |||
| import 'storybook-addon-react-docgen/register'; | |||
There was a problem hiding this comment.
main.js в "новых" рекомендациях указано проводить настройки в данный файл, давай сделаем все в одном стиле?
There was a problem hiding this comment.
| @@ -23,7 +23,21 @@ module.exports = { | |||
| ], | |||
| enforce: 'pre', | |||
| }); | |||
|
|
|||
| config.module.rules.push({ | |||
There was a problem hiding this comment.
А это заработало?
// 2b. Run `source-loader` on story files to show their source code
// automatically in `DocsPage` or the `Source` doc block.
https://github.com/storybookjs/storybook/tree/master/addons/docs#manual-configuration
There was a problem hiding this comment.
у нас это и так есть
поэтому я не настраивал
Или там какие-то особые плюшки будут?
There was a problem hiding this comment.
Можно будет посмотреть исходный код прямо в addon )
There was a problem hiding this comment.
ну я подключил, но видимо из-за того что docs не подключен - никаких изменений не вижу
There was a problem hiding this comment.
подключил - но плюсов не увидел, если поможешь с mdx будет круто )
| @@ -1 +1,5 @@ | |||
| import 'loki/configure-react'; | |||
| import { addDecorator } from '@storybook/react'; | |||
There was a problem hiding this comment.
Можно обойтись без декораторов
https://github.com/storybookjs/storybook/tree/master/addons/docs#manual-configuration
There was a problem hiding this comment.
это другой аддон, на сколько я вижу
у меня используется https://www.npmjs.com/package/storybook-addon-react-docgen
There was a problem hiding this comment.
Я конечно не настаиваю, хотя популярность и возможности сильно отличаются )
https://github.com/storybookjs/storybook/tree/master/addons/docs
There was a problem hiding this comment.
честно - я потратил часов 5, но mdx у меня так и не заработал ))
я помню, что ты его хотел
__mocks__/styleMock.js
Outdated
| @@ -0,0 +1,3 @@ | |||
| // https://jestjs.io/docs/en/webpack#handling-static-assets | |||
There was a problem hiding this comment.
Можно использовать директорию internals для такого рода настроек и конфигов
| @@ -0,0 +1,20 @@ | |||
| .cell { | |||
There was a problem hiding this comment.
Предлагаю сделать как обсуждали
- перенести калькулятор из директории
src/lesson2на уровень выше и назвать эту директорию чем то вродеcalc - удалить все тестовые компоненты из
src - добавить код в директорию
src/components
Дальше будем разрабатывать шаг за шагом приложение)
There was a problem hiding this comment.
Тогда сначала нужно поставить теги и обновить README (потому что это поломает примеры, которые ты давал)
There was a problem hiding this comment.
Согласен, с меня теги по первым урокам)
По поводу обновления readme, у нас же там ссылка на pull request, соответственно изменения ничего не поломают, можно будет переключиться по истории изменений на нужное состояние.
Так же можно открыть сам pull request и посмотреть изменения
src/lesson4/Field.tsx
Outdated
| x={x} | ||
| y={y} | ||
| onClick={props.onClick} | ||
| key={`${x}_${y}`} |
There was a problem hiding this comment.
Может стоит сделать key первым свойством?
There was a problem hiding this comment.
оно сделано последним, потому что он нем речи еще не было
src/lesson4/Field.tsx
Outdated
|
|
||
| export default Field; | ||
|
|
||
| export function getField(props: FieldProps) { |
There was a problem hiding this comment.
Short arrow functions notation ? export getField = () => <Field {...props} />
| ...row.map((filled, x) => { | ||
| return getCell({ | ||
| onClick: props.onClick, | ||
| filled: filled ? filled : undefined, |
There was a problem hiding this comment.
filled?: string; насколько я вижу из интерфейса, почему бы не использовать это?
в этом случае, если он потенциально undefined
{
onClick: props.onClick,
filled,
//...
There was a problem hiding this comment.
это сделано чтобы пустая строка обрабатывалась как undefined
| /** | ||
| * x-size for field | ||
| */ | ||
| xSize: number; |
There was a problem hiding this comment.
sizes: { x: number, y: number } ?
There was a problem hiding this comment.
ну тут тот же аргумент что и с x,y (хотя здесь мы вроде менять не собираемся часто)
src/lesson4/InteractiveField.tsx
Outdated
|
|
||
| public onClick(x: number, y: number) { | ||
| if ( | ||
| y >= 0 && |
There was a problem hiding this comment.
Кажется, стоит сделать логическую переменную с читаемым названием
const playerCanPutMark = y >= 0 &&
y < this.state.fieldState.length &&
x >= 0 &&
x < this.state.fieldState[0].length`
README.md
Outdated
| [Pull request](https://github.com/nickovchinnikov/react-js-tutorial/pull/9) | ||
|
|
||
| [Presentation](https://docs.google.com/presentation/d/1TVkOviAfORwmYncUL51g2UboNtj8kczWoLIRL3hO7AE/edit?usp=sharing) |
There was a problem hiding this comment.
А можем презентации хранить в одном спейсе?
Можешь переместить на нашу общую папку, плиз?
Что сделано: