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

Решение задачи "Отмеченные Email-ы" #6

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

Conversation

eastmann
Copy link

No description provided.

@jsru-1
Copy link
Contributor

jsru-1 commented Oct 14, 2024

Проверьте, пожалуйста, ваше решение, не все тесты прошли (PR не будет принят до тех пор, пока все добавленные задачи не будут решены).

@jsru-1
Copy link
Contributor

jsru-1 commented Oct 14, 2024

Добавляю преподавателя (@ShGKme) для код-ревью.

@jsru-1 jsru-1 requested a review from ShGKme October 14, 2024 07:32
@eastmann
Copy link
Author

eastmann commented Oct 14, 2024

Коммит решения задачи "Отмеченные Email-ы".

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Извиняюсь за долгую проверку, пропустил что разделено на два PR.

Во втором проблем нет - перенесите решение из него в эту ветку, чтобы была полностью рабочая ветка.

Comment on lines +11 to +34
const result = ref('')

watchEffect(() => {
switch (action.value) {
case 'sum':
result.value = firstOperand.value + secondOperand.value
break

case 'subtract':
result.value = (firstOperand.value - secondOperand.value) || ''
break

case 'multiply':
result.value = (firstOperand.value * secondOperand.value) || ''
break

case 'divide':
result.value = (firstOperand.value / secondOperand.value) || ''
break

default:
break
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Здесь через отслеживание вручную реализовано вычисление result при изменении любой из зависимостей вычисления. Фактически реализовано вычисляемое свойство, но вручную. Такие свойства нужно реализовывать именно как вычисляемое свойство computed:

  • Это явно. Так result будет описан именно как вычисляемый, а не как самостоятельная переменная с собственным значением
  • Эффективнее - вычисляемые свойства ленивые и вычисляются только, когда к их значению обращаются

emails.forEach((email, i) => {
const condition = input.value && email.includes(input.value)

arr[i] = condition ? 'marked' : ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Привязку класса лучше оставить в шаблоне, чтобы в JS части осталась логика работы компонента и его данных, включая флаг маркировки, а название класса для вёрстки - в шаблоне. Так в шаблоне будет явно видно вёрстку, какие на элементе классы и как они определяются.

Copy link
Contributor

Choose a reason for hiding this comment

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

Для этого отлично подойдёт привязка класса по условиям объектом.

https://vuejs.org/guide/essentials/class-and-style.html#binding-to-objects

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