Conversation
Closes #500
www/public/app/js/modules/search.js
Outdated
|
|
||
| this.elements.closer && this.elements.closer.addEventListener('click', () => this.hide()); | ||
|
|
||
| const delayedSearch = codex.core.throttle( |
There was a problem hiding this comment.
тут нужно использовать debounce, а не throttle
www/public/app/js/modules/search.js
Outdated
| const MIN_SEARCH_LENGTH = 3; | ||
| const SEARCH_TIMEOUT = 500; | ||
|
|
||
| const search = { |
|
|
||
| if ($success) { | ||
| /** | ||
| * Return Model_Page[] to user |
| position: absolute; | ||
| box-sizing: border-box; | ||
| z-index: 6; | ||
| top: 143px; |
There was a problem hiding this comment.
это точно на всех экранах будет норм выглядеть? может в vh задать?
There was a problem hiding this comment.
Помнится, в чате была пачка комментов по UI. Займусь ими и это заодно пофикшу
| margin-left: -30px; | ||
| border-radius: 50%; | ||
| border: 3px solid #ccc; | ||
| border-top-color: #7b8999; |
There was a problem hiding this comment.
вынеси цвета в переменные прямо в этом файле
www/public/app/js/modules/search.js
Outdated
|
|
||
| /** | ||
| * Perform search on user input | ||
| * @param value |
www/public/app/js/modules/search.js
Outdated
| if (this.elements.modal) { | ||
|
|
||
| this.elements.modal.removeAttribute('hidden'); | ||
| document.body.style.overflow = 'hidden'; |
www/public/app/js/modules/search.js
Outdated
|
|
||
| const loader = document.createElement('div'); | ||
|
|
||
| loader.classList.add('loader'); |
| * @returns {HTMLDivElement} | ||
| */ | ||
| createLoader() { | ||
|
|
www/public/app/js/modules/search.js
Outdated
| /** | ||
| * Adjust related DOM elements appearance | ||
| */ | ||
| this.elements.placeholder.setAttribute('hidden', true); |
There was a problem hiding this comment.
снова какие-то стили хардкодятся. Это все должно быть в методах
There was a problem hiding this comment.
Вынести в метод не сложно. А это не будет over-engineering? Метод будет вызван всего один раз именно с этими параметрами
There was a problem hiding this comment.
Хотя в принципе можно и стили через setAttribute добавлять. Тогда будет смысл
There was a problem hiding this comment.
чтобы скрыть плейсхолдер js вообще можно не использовать. Это можно сделать на css.
www/public/app/js/modules/search.js
Outdated
| /** | ||
| * Adjust related DOM elements appearance | ||
| */ | ||
| this.elements.placeholder.setAttribute('hidden', true); |
There was a problem hiding this comment.
чтобы скрыть плейсхолдер js вообще можно не использовать. Это можно сделать на css.
www/public/app/js/modules/search.js
Outdated
| * Adjust related DOM elements appearance | ||
| */ | ||
| this.elements.placeholder.setAttribute('hidden', true); | ||
| this.elements.searchResults.removeAttribute('hidden'); |
There was a problem hiding this comment.
зачем скрывается блок с результатами? это и есть оверинжениринг.
There was a problem hiding this comment.
Юз-кейс: пользователь закрыл модалку поиска и открыл заново — поисковой выдачи быть не должно. Можно убрать элемент из DOM, можно скрыть при помощи CSS, повесив класс на родителя. Это всего лишь один из способов.
There was a problem hiding this comment.
надо очищать выдачу, а не удалять обертку
There was a problem hiding this comment.
В принципе норм — можно сделать три стейта:
- Поле поиска пустое (либо меньше трех символов введено) — поисковая выдача пустая
- Поле поиска не пустое — поисковая выдача есть
- Поле поиска не пустое — в поисковой выдаче сообщение, что результатов нет
Если поле было не пустое, а пользователь его стер, снова поисковая выдача пустая, без заглушки.
3 participants
resolve #500