Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Добавить чтение параметров #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mirout
Copy link
Collaborator

@mirout mirout commented Jun 18, 2023

Так я сделал какой-то набросок по вводу параметров,
В принципе я добавил компонент для ввода и три валидатора для string, number и array.
Опыта особо с ts нет, поэтому не особо знаю всяких его соглашений и тп, так что можно смело ревьюить по полной

#1

@mirout mirout changed the title Add parameters MVP Добавить чтение параметров Jun 18, 2023
error: boolean
}

export class ParameterComponent<T> extends React.Component<ParameterProps<T>, State<T>> {
Copy link
Owner

Choose a reason for hiding this comment

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

Классовые компоненты - зло.
Давай перепишем на функциональные?
А ещё кажется что это не должно быть в lib
в lib скорее должно быть всё что нужно для алгоритмов
Давай компоненты для отрисовки вынесем в папочку components и будем делать по одному семантически важному компоненту в файле?

Copy link
Owner

Choose a reason for hiding this comment

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

Да, парадигма реакта сложная
Но в то же время достаточно простая и минималистичная.
Нужно просто немного по-другому думать.
Если нужно - я могу это доделать

Copy link
Owner

Choose a reason for hiding this comment

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

  • если мы вынесем это из lib то можно будет более логично описать стили для того чтобы сделать красиво

return <div>
<button onClick={() => doStart([[5, 4, 3, 2, 1]], false)}>Start</button>
<button onClick={() => doStart([[5, 4, 3, 2, 1]], true)}>Run Full</button>
<ul>{components}</ul>
Copy link
Owner

Choose a reason for hiding this comment

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

Когда рисуешь список компонентов каждому нужно указывать key=...


const propArr = {
label: "массив",
value: new Arr<number, Num>(new Num())
Copy link
Owner

Choose a reason for hiding this comment

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

Семантически непонятно - зачем передавать сначала number - тип, а потом Num - класс который по сути боксит значение при валидации
Может переименуем классы в ValidNum или SafeNum или во что то другое?

value: new Arr<number, Num>(new Num())
}

const components = [<ParameterComponent<number[]> {...propArr} />].map((v) => <li>{v}</li>);
Copy link
Owner

Choose a reason for hiding this comment

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

Если честно, то такая конструкция мне немного режет глаза :)
Давай перенесем map в return?
Или сделаем компонент для того чтобы рендерить несколько параметров через children?
Там же можно будет и стили накидать

@irdkwmnsb
Copy link
Owner

Отличная работа!

@irdkwmnsb
Copy link
Owner

Переоткрываю PR чтобы стриггерить деплой на github pages

@irdkwmnsb irdkwmnsb closed this Jun 19, 2023
@irdkwmnsb irdkwmnsb reopened this Jun 19, 2023
@irdkwmnsb
Copy link
Owner

Ещё раз

@irdkwmnsb irdkwmnsb closed this Jun 19, 2023
@irdkwmnsb irdkwmnsb reopened this Jun 19, 2023
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