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

Задачи CreateApp, WeatherApp, CounterApp и MapApp #3

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

slenshin
Copy link
Contributor

@slenshin slenshin commented Oct 4, 2024

В задачу CreateApp внёс поправки по вашим рекомендациям

@jsru-1
Copy link
Contributor

jsru-1 commented Oct 4, 2024

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

@jsru-1 jsru-1 requested a review from ShGKme October 4, 2024 11:58
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.

Принято

Comment on lines +19 to +26
const timeToTimestamp = (time) => {
return Date.parse('1970-01-01T' + time)
}

const isNignt = (dt, sunrise, sunset) => {
const ts = timeToTimestamp(dt);
return ts < timeToTimestamp(sunrise) || ts > timeToTimestamp(sunset)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Строки в формате HH:MM можно сравнивать просто лексикографически как строки без преобразований. '08:30' < '09:12'

Comment on lines +44 to +47
:class="{
'weather-card': true,
'weather-card--night': isNignt(row.current.dt, row.current.sunrise, row.current.sunset)
}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Статические классы, которые всегда есть у элемента, можно описывать явно в шаблоне через обычный атрибут class="weather-card". Так будет видно и основные классы верстки, и какие классы определяются динамически.

Comment on lines +70 to +75
<div class="weather-details__item" v-for="details in [
{label: 'Давление, мм рт. ст.', value: pressureHpaToMmhg(row.current.pressure)},
{label: 'Влажность, %', value: row.current.humidity},
{label: 'Облачность, %', value: row.current.clouds},
{label: 'Ветер, м/с', value: row.current.wind_speed},
]">
Copy link
Contributor

Choose a reason for hiding this comment

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

Лучше такие большие значения выносить в setup

@jsru-1 jsru-1 merged commit b336a7d into js-tasks-ru:master Oct 4, 2024
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