Skip to content

Conversation

@vvscode
Copy link
Collaborator

@vvscode vvscode commented Apr 24, 2020

No description provided.

@vvscode vvscode requested a review from nickovchinnikov April 24, 2020 12:45
symbol: "Y",
},
player2: {
name: "",
Copy link
Owner

Choose a reason for hiding this comment

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

Классика name: "Alice" =)


export const forms = () => (
<StoryWrapper>
<DemoForm FormComponent={GameSettingsFormDOM} title="GameSettingsFormDOM" />
Copy link
Owner

Choose a reason for hiding this comment

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

FormComponent -> children?

<DemoForm title="GameSettingsFormRef">{GameSettingsFormRef}</DemoForm>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

см выше - DemoForm не просто для визуализации обертка

margin-right: 10px;
`;

const DemoForm: React.FC<DemoFormProps> = ({ FormComponent, title }) => {
Copy link
Owner

Choose a reason for hiding this comment

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

const DemoForm: React.FC<DemoFormProps> = ({ children: FormComponent, title }) => {

Copy link
Collaborator Author

@vvscode vvscode Apr 24, 2020

Choose a reason for hiding this comment

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

я еще пробрасываю ref и callback - тогда children придется переименовывать (компонент должен быть с большой буквы)

те

{children} // ok

<Children onSubmit={} /> // не ок

Copy link
Owner

Choose a reason for hiding this comment

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

children: FormComponent вроде смапит имя или нет?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

смапит, но это же не children )) children (по крайней мере в моей практике) - дочерние элементы, которые рендерятся как есть

Copy link
Owner

Choose a reason for hiding this comment

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

Хорошо, принял + понял )

@@ -0,0 +1,91 @@
import React from "react";
import { GameSettingsFormProps } from "./interfaces";

Copy link
Owner

Choose a reason for hiding this comment

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

const getInputValue = (target: HTMLFormElement, name: string) => 
  (target.querySelector(`[name=${name}]`) as HTMLInputElement).value

//...

   this.props.onSubmit({
      player1: {
        name: getInputValue("player1Name")
//...


render() {
return (
<form onSubmit={this.handleSubmit}>
Copy link
Owner

Choose a reason for hiding this comment

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

dump component form?

Copy link
Collaborator Author

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.

Я имел в виду что то вроде того

      export const Player = ({ children: playerName, ... }) <fieldset>
          <legend>Game Settings</legend>
          <fieldset>
            <legend>{playerName}</legend>
            <label>
              Name:
              <input
                name={playerName}
                type="text"
                placeholder={`${playerName} name`}
                required
              />
            </label>
//...

<form onSubmit={this.handleSubmit}>
  <Player prop1="some" ...>Player1</Player>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

в реальном проекте (ну кроме того, что я бы так не писал - возможно да)

но тут через все компоненты я стараюсь провести одну структуру (с минимальными изменениями, кроме подхода к работе с данными)

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

Copy link
Owner

Choose a reason for hiding this comment

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

А если прокинуть ref или обработчик сверху? Думаю привевать композицию с самого начала студентам надо )


render() {
return (
<form onSubmit={this.handleSubmit}>
Copy link
Owner

Choose a reason for hiding this comment

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

  • dump component form ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

тоже что и выше )

import React from "react";
import { GameSettingsFormProps } from "./interfaces";

interface GameSettingsFormStateState {
Copy link
Owner

Choose a reason for hiding this comment

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

Proposal

Можно было бы везде использовать один тип, тоесть возможно его стоило бы разместить в src/types

export interface Player {
 name: string;
 color: string;
 symbol: string;
}

И стейт

interface GameSettingsFormState {
  player1: Player;
  player2: Player;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Долистал до https://github.com/nickovchinnikov/react-js-tutorial/pull/14/files#diff-fc42a5bbd7418b6789d5ac5e3c143976
А почему его не использовать бы тут?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

потому что это эволюционное изменение прошлых компонент - чтобы показать разницу с минимальными изменениями (все идет к Formik, где так и есть)

Copy link
Owner

Choose a reason for hiding this comment

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

ИМХО структура данных может быть не привязана к view

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

полностью согласен, НО (=) ):

в примере с DOM / REF струтура плоская (что определяется самой формой хранения данных - plain html), и здесь я не хочу перескакивать ступеньку (один шаг - переход на state, второй шаг - держать вложенные данные в стейте )

Могу просто сделать еще одну форму для этого шага

this.setState(
{
[prop]: (ev.target as HTMLInputElement).value,
} as any /** don't do this in real projects. Better to have multiple setters */
Copy link
Owner

Choose a reason for hiding this comment

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

as GameSettingsFormState ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

это не совместимые типы -только если через unkown кастовать, тут меня вообще очень сильно удивили тайпинги для setState - он даже Partial<GameSettingsFormStateState> не хочет принимать

Copy link
Owner

Choose a reason for hiding this comment

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

Странно, действительно (


render() {
return (
<form onSubmit={this.handleSubmit}>
Copy link
Owner

Choose a reason for hiding this comment

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

dump component form ?
я думаю, это можно сделать "глупыми" компонентом для всех случаев
тут просматривается несколько компонентов Player, можно красиво скомпозировать ИМХО

@vvscode vvscode merged commit 2d24063 into master Apr 25, 2020
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.

3 participants