Skip to content

Lesson 4 react component JSX#2

Open
mkremnev wants to merge 3 commits into
masterfrom
lessons/lesson4
Open

Lesson 4 react component JSX#2
mkremnev wants to merge 3 commits into
masterfrom
lessons/lesson4

Conversation

@mkremnev

@mkremnev mkremnev commented Aug 1, 2020

Copy link
Copy Markdown
Owner

No description provided.

Comment thread src/index.tsx
@@ -0,0 +1,2 @@
import React from 'react';
import { render } from 'react-dom';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

что это?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Этого не должно быть здесь, перекину в векту master, предпологалось как шаблон использовать

Comment thread src/lesson4/Cell.tsx Outdated

export default Cell;

export function getCell(props: CellProps) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

не вполне понятно зачем эта функция тут ( в тестах ее можно инлайново определить)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Принял, сделаю инлайново

Comment thread src/lesson4/Cell.stories.tsx Outdated
};

export const nonFilled = () => [
<Cell x={number('x', 1)} y={number('y', 23)} key="nonFilled" />,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

а зачем тут key?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Косячнул, уберу. Здесь не нужен.

Comment thread src/lesson4/Field.stories.tsx Outdated
export const emptyCell = () => [
<Field
field={object('field', [
[

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

хорошо бы Array.from({length: 5}).fill(false) или вроде того, а то читать не особо понятно - еще и шанс ошибиться в числе элементов выше

@mkremnev mkremnev Aug 4, 2020

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Сделаю тогда вот так
Array.from<boolean[]>({ length: 10 }).fill( Array.from<boolean>({ length: 10 }).fill(false), );

const cellGridFillRandom = (
rows: number,
columns: number,
cellStatus = () => Math.random() < 0.3,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

а зачем ты эту функцию несколько раз инлайново определяешь?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Вроде один раз же определил в этом компоненте? Или я недопонял вопроса(

Comment thread src/lesson4/GameOfLifeProto.tsx Outdated
this.state = {
fieldState: cellGridFillRandom(this.props.rows, this.props.columns),
};
this.onClick = this.onClick.bind(this);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

как на счет поставить бабель плагин и не делать это руками?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Это который вот этот @babel/plugin-proposal-class-properties?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

И мне тогда получается можно использовать методы компонента через стрелочные функции, правильно понимаю?


public onClick() {
this.setState(() => {
const cloneFieldState = cellGridFillRandom(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

на сколько я вижу, здесь коллбэк не обязателен

field={this.state.fieldState}
onClick={this.onClick}
/>
<button className="btn" onClick={() => this.onClick()}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ты в конструкторе завязал контекст у метода - для чего?

я к тому, что тут можно обойтись без стрелочной функции

@mkremnev mkremnev Aug 10, 2020

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Я заготовку начал делать видимо для метода, который будет при клике возвращать координаты ячейки и видимо забыл убрать, я уберу эту привязку

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.

2 participants