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

небольшие комментарии #59

Open
atherdon opened this issue Mar 21, 2021 · 2 comments
Open

небольшие комментарии #59

atherdon opened this issue Mar 21, 2021 · 2 comments

Comments

@atherdon
Copy link
Collaborator

atherdon commented Mar 21, 2021

  1. можем ли мы вынести этот большой объект outside - мне просто ненравится когда внутри компонента все лежит.
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L14

пример из туториала который я нашел

const LoginSchema = Yup.object().shape({
  email: Yup.string()
    .email("Invalid email address format")
    .required("Email is required"),
  password: Yup.string()
    .min(3, "Password must be 3 characters at minimum")
    .required("Password is required")
});

<Formik
              initialValues={{ email: "", password: "" }}
              validationSchema={LoginSchema}
  1. сделать отдельным компонентом. можем ли мы заюзать компонент, который мы делали в sendgrid-forms? https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L53

  2. использовать email component для обоих форм

  3. вынести чекбоксы в отдельные файлы. я хочу потом эти checkboxes re-use
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L86
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L92
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L98
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L104
    https://github.com/atherdon/subscribe-form/blob/master/components/forms/subscribe.js#L110

сс @coder-do

@coder-do coder-do mentioned this issue Apr 7, 2021
Merged
@coder-do
Copy link
Contributor

coder-do commented Apr 7, 2021

Первые 3 сделал

насчёт 4ого - у нас и так есть отдельный компонент чекбоксов, если имеете ввиду что еще эти 5 вместе вынести куда то - сделаю

@atherdon
Copy link
Collaborator Author

atherdon commented Apr 7, 2021

ну у нас хоть и компонент отдельный, все равно параметры передаются тут.

причина по которой я хочу вынести их отдельно - чтобы форма была более flexible. плюс позже, мы сможем это использовать в #56

там же тоже есть чекбоксы, которые относятся к типам рассылок

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

No branches or pull requests

2 participants