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

bucket list, prod by m.v.novikov88 #1

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

Conversation

m-v-novikov
Copy link

No description provided.

Copy link
Member

@EvgenyOrekhov EvgenyOrekhov left a comment

Choose a reason for hiding this comment

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

Пояснения по оригинальным требованиям к тестовому заданию.

Use React with Hooks

Подразумевает, что все компоненты должны быть реализованы на Hooks, а не на классах.

Use styled-components

Подразумевает, что вся стилизация должна быть произведена с помощью styled-components, а не с помощью отдельных файлов со стилями.

src/js/actions/toast.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved
const validation = {};

this.validations.map(rule => (
validation[rule["field"]] = { isInvalid: false, message: '' }
Copy link
Member

Choose a reason for hiding this comment

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

Возможно было бы лучше использовать положительное слово, isValid, например. Так как дальше по коду производится проверка !isInvalid, а отрицание отрицания уже становится тяжеловато читать.


function handleSortActivities(type) {
const sortedActivityArr = activitiesArr.sort((a, b) => {
if (a[type] < b[type]) return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Фигурные скобки лучше никогда не опускать, даже если инструкция всего одна. Ужасные баги можно бы было предотвратить следуя этому правилу (см. "goto fail bug" от Apple в их библиотеке SSL).

src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesList.js Outdated Show resolved Hide resolved
src/js/components/common/layout/ActivitiesForm.js Outdated Show resolved Hide resolved

const mapState = (state) => ({
_app: state.app
Copy link
Member

Choose a reason for hiding this comment

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

Слишком много идентификаторов с префиксом _ появилось. Лучше давать более осмысленные имена.

Copy link
Author

@m-v-novikov m-v-novikov Jan 28, 2019

Choose a reason for hiding this comment

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

да это есть, префикс я начал давать для actions(и для других некоторых переменных например как toast) т.к. функция(имя переменной) одна и та же(чтоб не было конфликтов имен). Тут да - надо что то придумать и придерживаться этого правила.

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