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

first commit #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

first commit #1

wants to merge 1 commit into from

Conversation

iRussov
Copy link
Owner

@iRussov iRussov commented May 19, 2024

No description provided.

Copy link

@tvivls tvivls left a comment

Choose a reason for hiding this comment

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

при переходе на страницу таймер отображается не сразу и высота контейнера зависит от его наличия.
поправь верстку, чтобы соответствовала макету (шрифты, отступы и тд);
нужно, чтобы роутинг был без хэшей #;
при обновлении неверно отображается активная страница;
при переходе между страницами с помощью кнопок назад/вперед в браузере, не изменяется подсветка активной страницы;

Исправишь - тегни меня здесь

Copy link

Choose a reason for hiding this comment

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

дублируешь иконки
можно экспортировать все иконки из фигмы
используй svg формат, он легче

@@ -0,0 +1,228 @@
const mapPage = `
Copy link

Choose a reason for hiding this comment

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

интересный подход, но мб вынести html к html? в js файлах оставить только js


hours = (hours < 10) ? "0" + hours : hours;
minutes = (minutes < 10) ? "0" + minutes : minutes;
seconds = (seconds < 10) ? "0" + seconds : seconds;
Copy link

Choose a reason for hiding this comment

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

можно как-нибудь избежать дублирования?)

}

function initializeMap() {
setTimeout(() => {
Copy link

Choose a reason for hiding this comment

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

зачем тут setTimeout?

navigateTo('/timer');
});

document.querySelectorAll('nav ul li').forEach(function(item) {
Copy link

Choose a reason for hiding this comment

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

выше везде писал через () => {
выбери один подход

@@ -0,0 +1,15 @@
.container, .row{
max-width: 760px;
Copy link

Choose a reason for hiding this comment

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

задавай размеры за счет margin/padding + у тебя же адаптив должен быть

Copy link

Choose a reason for hiding this comment

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

еще я не могу понять откуда ты берешь иконки?))
экспортируй из фигмы

Copy link

Choose a reason for hiding this comment

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

можно просто использовать теги <link> и <script> (см. доку bootstrap)

Copy link

Choose a reason for hiding this comment

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

тут тоже просто <link> и <script>

<i class="fas fa-camera me-1"></i> <span>Photos</span>
</div>
<div class="icon-text-group">
<i class="fas fa-cog me-1"></i> <span></span>
Copy link

Choose a reason for hiding this comment

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

зачем пустой span?

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