Skip to content

Conversation

@genius192x
Copy link
Contributor

No description provided.

@jsru-1
Copy link
Contributor

jsru-1 commented May 4, 2025

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

@jsru-1 jsru-1 requested a review from ShGKme May 4, 2025 19:15
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.

Принято, но обратите внимание на комментарии

<UiInput v-model.trim="query" type="search" placeholder="Поиск" aria-label="Поиск" small />
</UiFormGroup>
<EmailList :emails="markedEmails" />
<EmailList :emails="markedEmails" @remove="(value)=>{removeEmailByIndex(value)}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

В директиву v-on можно передавать сразу тело обработчика события, а параметр получать из $event.

@remove="removeEmailByIndex($event)"

А так как просто вызывается другая функция с этим параметром - можно просто передать саму функцию

@remove="removeEmailByIndex"

Comment on lines +32 to +35
<UiCounter v-model:count="count1" @update:count="(value)=>{count1 = value}"/>
</p>
<p style="margin: 1em 0">
<UiCounter v-model:count="count2" :min="1" :max="3" />
<UiCounter v-model:count="count2" :min="1" :max="3" @update:count="(value)=>{count2 = value}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Обработка @update:count здесь не нужна, так как она уже выполняется в директиве двустороннего связывания v-model:count

Comment on lines +35 to +37
function update(operation) {
operation == 'inc' ? emit('update:count', props.count + 1) : emit('update:count', props.count - 1)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Тернарный оператор здесь используется не совсем корректно. Это именно оператор (как a + b), который выполняет вычисление и возвращает результат - 2-ой или 3-ий операнд. Здесь оба вычисляемых значения - undefined (так как emit()) не возвращает значения. А результат всего оператора не используется.

Более корректным будет либо использовать ветвление:

if (operation == 'inc') {
  emit('update:count', props.count + 1)
} else {
  emit('update:count', props.count - 1)
}

Либо использовать тернарный оператор для вычисления действительно значения

emit('update:count', operation == 'inc' ? props.count + 1 : props.count - 1)

Comment on lines +9 to +13
data() {
return {
weatherData: getWeatherData(),
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Это не Composition API 👀

Comment on lines +16 to +40
computed: {
weatherIcon() {
return WeatherConditionIcons[this.data.current.weather.id]
},

temperature() {
return (this.data.current.temp - 273.15).toFixed(1)
},

details() {
return {
'Давление, мм рт. ст.': (this.data.current.pressure * 0.750062).toFixed(0),
'Влажность, %': this.data.current.humidity,
'Облачность, %': this.data.current.clouds,
'Ветер, м/с': this.data.current.wind_speed,
}
},

isNight() {
const currentTime = this.data.current.dt
const sunriseTime = this.data.current.sunrise
const sunsetTime = this.data.current.sunset
return currentTime < sunriseTime || currentTime > sunsetTime
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

И это 👀

@jsru-1 jsru-1 merged commit 5006cd8 into js-tasks-ru:master May 5, 2025
1 check passed
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