Jump to content

Оцените, пожалуйста, качество верстки


Victor-ru
 Share

Recommended Posts

Здравствуйте. Прошу оценить верстку и указать на ошибки. 

Макет без адаптивности. Верстал по размерам из фотошопа.

index.html

main.css

сайт на хосте - http://www.testwebtest.h1n.ru/

Edited by Victor-ru
Link to comment
Share on other sites

спасибо за ответ))) В макете был статичный размер 1900рх. Как я понял это только вариант под такие экраны. А по коду какие нибудь замечания есть? 

Link to comment
Share on other sites

2 часа назад, Victor-ru сказал:

 Прошу оценить верстку и указать на ошибки

 

В блоке columns можно было сделать justify-content:space-around; и добавить паддинги, а не сдвигать элементы позиционированием.

Избегайте использовать фиксированную высоту. Что будет с версткой если изменятся размеры или количество внутренних элементов?

Не используйте абсолютное позиционирование там где это не нужно всю вашу страницу можно было сверстать вообще без использования позиционирования. Если вы планируете дальше изучать адаптивную верстку то эту страничку будет очень тяжело адаптировать.

Link to comment
Share on other sites

39 минут назад, AlexZaw сказал:

Избегайте использовать фиксированную высоту.

Спасибо. А если вместо height: использовать min-height, для соответствия размерам макета. Будет ли такой вариант правильный?

Link to comment
Share on other sites

9 минут назад, Victor-ru сказал:

А если вместо height: использовать min-height

min-height уже получше, если вы уверены что элемент не будет меньше указанного размера. Какие-либо родительские элементы лучше вообще не ограничивать в размерах ни в максимальных ни в минимальных,  пусть они зависят от контента. А недостающие размеры можно добить паддингами и/или маржинами

Есть конечно случаи когда можно и нужно задавать размеры, но они не так часто встречаются.

Link to comment
Share on other sites

9 часов назад, Victor-ru сказал:

Исправил ошибки. Надеюсь правильно все понял)))

Было бы лучше, если бы вы исправленную версию файла стилей внедрили бы на сайт,  а не просто выложили сюда

Link to comment
Share on other sites

11 минут назад, Victor-ru сказал:

Обновил, теперь на хосте последняя версия

Уже лучше. Только для top-menu я бы все-таки не задавал ширину, тем более фиксированную, оно ведь может как увеличится, так и уменьшиться, да и использовать margin-left c таким значением тоже не есть гуд, лучше уж использовать float:right; и отодвинуть меню с помощью margin-right, тоже в принципе относится и к остальным элементам с такими отступами.

Для ul с классом buttons_location_menu можно убрать фиксированную ширину, сделать обертку, задать обертке display:inlline-block; и задать для menu-header  text-align:center; тогда список будет по центру.

Если вам нужно ограничить какой то элемент по ширине - старайтесь использовать max-width и паддинги, либо задавать ширину в процентах, это облегчит последующую адаптацию страницы.

На беглый взгляд замечаний больше нет. Почитайте про адаптив, что это, с чем его едят, как он делается. В дальнейшем пригодится, да и даже если вы пока не готовы замахнуться на адаптивность, все-равно будет полезно знать, хотя бы для того, что-бы не привыкать к фиксированным величинам и абсолютному позиционированию.

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

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

  • Similar Content

    • By zeiger2
      Здравствуйте! У меня стоит задача, что при наведении на блок li строка должна поменять цвет, в том числе и картинка. Я меняю картинку с помощью 
      background-image: none;     background: url(../img/check_icon_red.png) left no-repeat;   Но теперь картинка позицианируется не там где должна, её можно поставить на место только вручную, через -100px. Нужно поставить ровно туда, где она была. Должна быть в одном ряду с другими
    • By Mix9
      есть див с 5 img, при уменьшении экрана див выходит за него. Я добавил overflow: auto для этого div в надежде на то, что я смогу прокручивать фотки с помощью скроллбара, однако даже с ним почему-то я не вижу часть фоток которые вышли за границу. Что с этим можно сделать? класс video повторяется 5 раз, я тут оставил только 1 
      .content{ width: 90%; background-color: #333; } .video{ margin: 0px 4px 0px 4px; width: 310; display: flex; flex-direction:column; } .video_button_text{ margin-top: 10px; display: flex; flex-direction: row; font-size: 20px; color: white; } .video_text_div{ display: inline-block; width: 250px; } .video_text{ text-align: justify-all; margin: 0px; display: -webkit-box; -webkit-line-clamp: 2; -webkit-box-orient: vertical; overflow: hidden; } .slidan_videos{ margin: 0px 10px 0px 20px; overflow: auto; width: auto; margin-bottom: 50px; display: flex; flex-direction: row; justify-content: space-around; } <div class="content"> <div class = slidan_videos> <div class = video> <div> <a href = 'ссылка'><img class="img" src=""картинка"></a> </div> <div class = video_button_text> <div class = avatarka_div> <a href="ссылка" target="_blank"><img class = avatarka src="картинка"></a> </div> <div class = video_text_div> <p class = video_text><a href="ссылка">текст</a></p> </div> </div> </div>
    • By Марко
      Добрый день. Начинающий программист, столкнулся с проблемой. Селектор .class не работает должным образом. Несмотря на правильное, я надеюсь, описание, на web-странице не отображается ни одно изображение. С чем может быть связано? Заранее спасибо за помощь. 



    • By Kaido
      Использую готовый плагин для модальных окон(от MaxGraph). Проблема в том, что когда у меня открыто два модальных окна, для примера Форма + Политика конфендициальности, и мне нужно закрыть политику вместе с ней закрывается и другое модальное окно. В JS я не сильно разбираюсь(собственно из за этого и использую готовый плагин), можете помочь кто работал с этим плагином? Я примерно понимаю как он работает, но реализовать чтобы закрывалось только одно не получается.
       
        <div class="content"> <button class="modal-btn" data-path="first" data-animation="fadeInUp" data-speed="1500">Открыть окно 1</button> </div> <div class="modal"> <div class="modal__wrapp" data-target="first"> <div class="modal__content"> <button class="modal__close">Закрыть</button> модальное окно <button data-path="policy">Политика</button> </div> </div> <div class="modal__wrapp" data-target="policy"> <div class="modal__content"> <button class="modal__close">Закрыть</button> политика </div> </div> </div> .modal { --transition-time: 0.3s; position: fixed; left: 0; top: 0; right: 0; bottom: 0; z-index: 1000; cursor: pointer; overflow-y: auto; overflow-x: hidden; text-align: center; opacity: 0; visibility: hidden; transition: opacity var(--transition-time), visibility var(--transition-time); } .modal__wrapp { display: none; cursor: default; width: fit-content; height: fit-content; } .modal__content{ position: absolute; left: 500px; width: 500px; height: 500px; display: flex; color: white; flex-direction: column; text-align: left; background-color: #000; } .modal__content button{ width: 200px; height: 50px; margin: 50px 0; } .modal.is-open { opacity: 1; visibility: visible; transition: opacity var(--transition-time), visibility var(--transition-time); } .modal__wrapp.modal-open { display: flex; } .disable-scroll { position: relative; overflow: hidden; height: 100vh; position: fixed; left: 0; top: 0; width: 100%; } .fade { opacity: 0; transition: opacity var(--transition-time); } .fade.animate-open { opacity: 1; transition: opacity var(--transition-time); } .fadeInUp { opacity: 0; transform: translateY(vw(-100)); transition: opacity var(--transition-time), transform var(--transition-time); } .fadeInUp.animate-open { opacity: 1; transform: translateY(0); transition: opacity var(--transition-time), transform var(--transition-time); } .modal__wrapp[data-target="policy"] .modal__content{ left: 1050px; background-color: #000; opacity: .5; } class Modal { constructor(options) { let defaultOptions = { isOpen: () => {}, isClose: () => {}, } this.options = Object.assign(defaultOptions, options); this.modal = document.querySelector('.modal'); this.speed = false; this.animation = false; this.isOpen = false; this.modalContainer = false; this.previousActiveElement = false; this.fixBlocks = document.querySelectorAll('.fix-block'); this.focusElements = [ 'a[href]', 'input', 'button', 'select', 'textarea', '[tabindex]' ]; this.events(); } events() { if (this.modal) { document.addEventListener('click', function(e){ const clickedElement = e.target.closest('[data-path]'); if (clickedElement) { let target = clickedElement.dataset.path; let animation = clickedElement.dataset.animation; if (clickedElement.classList.contains('modal-close')) { this.close(); } let speed = clickedElement.dataset.speed; this.animation = animation ? animation : 'fade'; this.speed = speed ? parseInt(speed) : 300; this.modalContainer = document.querySelector(`[data-target="${target}"]`); this.open(); return; } if (e.target.closest('.modal__close')) { this.close(); return; } }.bind(this)); window.addEventListener('keydown', function(e) { if (e.keyCode == 27) { if (this.isOpen) { this.close(); } } if (e.keyCode == 9 && this.isOpen) { this.focusCatch(e); return; } }.bind(this)); this.modal.addEventListener('click', function(e) { if (!e.target.classList.contains('modal__wrapp') && !e.target.closest('.modal__wrapp') && this.isOpen) { this.close(); } }.bind(this)); } } open() { this.previousActiveElement = document.activeElement; this.modal.style.setProperty('--transition-time', `${this.speed / 1000}s`); this.modal.classList.add('is-open'); this.disableScroll(); this.modalContainer.classList.add('modal-open'); this.modalContainer.classList.add(this.animation); setTimeout(() => { this.options.isOpen(this); this.modalContainer.classList.add('animate-open'); this.isOpen = true; this.focusTrap(); }, this.speed); } close() { if (this.modalContainer) { this.modalContainer.classList.remove('animate-open'); this.modalContainer.classList.remove(this.animation); this.modal.classList.remove('is-open'); this.modalContainer.classList.remove('modal-open'); this.enableScroll(); this.options.isClose(this); this.isOpen = false; this.focusTrap(); } } focusCatch(e) { const focusable = this.modalContainer.querySelectorAll(this.focusElements); const focusArray = Array.prototype.slice.call(focusable); const focusedIndex = focusArray.indexOf(document.activeElement); if (e.shiftKey && focusedIndex === 0) { focusArray[focusArray.length - 1].focus(); e.preventDefault(); } if (!e.shiftKey && focusedIndex === focusArray.length - 1) { focusArray[0].focus(); e.preventDefault(); } } focusTrap() { const focusable = this.modalContainer.querySelectorAll(this.focusElements); if (this.isOpen) { focusable[0].focus(); } else { this.previousActiveElement.focus(); } } disableScroll() { let pagePosition = window.scrollY; this.lockPadding(); document.body.classList.add('disable-scroll'); document.body.dataset.position = pagePosition; document.body.style.top = -pagePosition + 'px'; } enableScroll() { let pagePosition = parseInt(document.body.dataset.position, 10); this.unlockPadding(); document.body.style.top = 'auto'; document.body.classList.remove('disable-scroll'); window.scroll({ top: pagePosition, left: 0 }); document.body.removeAttribute('data-position'); } lockPadding() { let paddingOffset = window.innerWidth - document.body.offsetWidth + 'px'; this.fixBlocks.forEach((el) => { el.style.paddingRight = paddingOffset; }); document.body.style.paddingRight = paddingOffset; } unlockPadding() { this.fixBlocks.forEach((el) => { el.style.paddingRight = '0px'; }); document.body.style.paddingRight = '0px'; } } const modal = new Modal({ isOpen: (modal) => { console.log(modal); console.log('opened'); }, isClose: () => { console.log('closed'); }, });  
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue. See more about our Guidelines and Privacy Policy