bugs/ui

[alagunoff]

0. Убери автопереключение у слайдеров. Ты же не картинки в слайд шоу выводишь. К тому же это мешает тестировать ui мне)
1. Зачем тут ссылки, которые никуда не ведут?
2. Не совсем понятен мотив совмещения значений слайдера в одном инпуте, лучше разделить. У каждого ползунка должен быть свой инпут и свое значение в нем
3. То, что сделал больше 2-ух ползунков это круто конечно, но у тебя больше чем 2 ползунка не имеет смысла быть в принципе, как я думаю, поэтому лучше убрать
4. Не понятно также зачем два инпута current values и initial values. У тебя должен быть 1 инпут, в который выводится значение при загрузке страницы и потом, когда ты слайдер таскаешь, у тебя должно меняться значение в этом же инпуте
5. Состояние слайдера должно меняться на лету. Т.е., при анфокусе с инпутов, при взаимодействии с checkbox
6. Не изменяется значение шага
7. Добавить возможность смены темы слайдера через демо панель
8. Добавить прогресс бар к слайдеру
9. Добавить шкалу значений к слайдеру. По клику на число, ползунок должен вставать на позицию того значения, по которому кликнули

[alanreidt]

1. Я собираюсь в дальнейшем страницы добавить.
2. Так как у меня количество ползунков произвольное, то это единственное подходящее решение.
3. Это довольно спорно — слайдер может выполнять роль шкалы, на которой нужно отметить несколько значений. Для наглядности, можешь посмотреть на пример с главной страницы noUiSlider.
4. Это сделано согласно требованиям к проекту:

1. Рядом с каждым слайдером разместить панель конфигурирования, чтобы можно было на лету менять параметры.
2. Рядом с каждым слайдером разместить инпут, в котором всегда будет синхронизировано значение слайдера — при изменении инпута на анфокус слайдер тоже меняет значение. И наоборот, при изменении слайдера, в инпут устанавливается сразу значение.

Я так понимаю это сделано для того, чтобы можно было проверить разную функциональность API. Панель конфигурации каждый раз создает новый экземпляр слайдера (что позволяет менять все опции) — это create. А одиночный инпут обновляет значение ползунков (вообще конкретной опции модели) — это update. Про анфокус инпута я забыл — исправлю.

5. Это все-таки форма, поэтому я настроил срабатывание на submit event. То есть, при нажатии на Enter на любом элементе, слайдер обновляется.
6. Значение изменяется, оно корректируется Моделью слайдера. А так как ты экспериментировал с промежутком 9, то и шаг подстраивался, чтобы это число делилось без остатка (1, 3, 9).
7. 7, 8, 9 пунктов нет в требованиях.

[alagunoff]

1. Я собираюсь в дальнейшем страницы добавить.
2. Так как у меня количество ползунков произвольное, то это единственное подходящее решение.
3. Это довольно спорно — слайдер может выполнять роль шкалы, на которой нужно отметить несколько значений. Для наглядности, можешь посмотреть на пример с главной страницы noUiSlider.
4. Это сделано согласно требованиям к проекту:

1. Рядом с каждым слайдером разместить панель конфигурирования, чтобы можно было на лету менять параметры.
2. Рядом с каждым слайдером разместить инпут, в котором всегда будет синхронизировано значение слайдера — при изменении инпута на анфокус слайдер тоже меняет значение. И наоборот, при изменении слайдера, в инпут устанавливается сразу значение.

Я так понимаю это сделано для того, чтобы можно было проверить разную функциональность API. Панель конфигурации каждый раз создает новый экземпляр слайдера (что позволяет менять все опции) — это create. А одиночный инпут обновляет значение ползунков (вообще конкретной опции модели) — это update. Про анфокус инпута я забыл — исправлю.

1. Это все-таки форма, поэтому я настроил срабатывание на submit event. То есть, при нажатии на Enter на любом элементе, слайдер обновляется.
2. Значение изменяется, оно корректируется Моделью слайдера. А так как ты экспериментировал с промежутком 9, то и шаг подстраивался, чтобы это число делилось без остатка (1, 3, 9).
3. 7, 8, 9 пунктов нет в требованиях.

1. Ок, тогда исправь по стандартам (14 пункт)
2. Посмотрел пример и все равно вопрос остался. Ты пишешь, что он может выполнять роль шкалы. Какой шкалы? Где она может пригодиться? Где ты видишь ее использование?
4. Хм, согласен. Я, к примеру, просто сделал одну панель, которая отображала текущее состояние слайдера и ты мог менять параметры в ней. Оставляй тогда так и исправляй анфокус.
5. Этот вопрос связан с предыдущим, поэтому думаю нормально, оставим.
6. Не верно сделано, у тебя неявное поведение получается, 9 это валидное значение шага и оно вписывается в рамки 1-10, поэтому я должен в этом случае выбирать между 1-9-10
7. Возвращаясь тогда к вопросу 2 и 3. Ты добавил выбор множества ползунков аргументируя это тем что "слайдер может(!) выполнять роль шкалы, на которой нужно отметить несколько значений", именно по этой причине у слайдера может быть (а может и не быть :)) несколько тем, шкала значений и прогресс бар. Ну а вообще - это best practices по созданию слайдера. Ну и это тебе поможет больше вникнуть в архитектуру плагина и получить опыт взаимодействия с композитной вьюхой

[alanreidt]

@madnessJs \1. Хорошо. \2. Примеры: деление отрезка на процентные соотношения; тоже самое; деление бюджета на каналы; \6. Ты неправильно понял концепцию шага. Это итерация одного и того же числа, на всем интервале (аналогично буквальным шагам, отсюда и название). То есть, 1 + 3 = 4 => 4 + 3 = 7 => 7 + 3 = 10. Либо, 1 + 9 = 10. \7. Чуть позже объясню.

[alanreidt]

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

А композитной архитектурой я вдоволь могу насладиться реализуя демо страницу (тем более, что она уже, практически, реализована в желаемом виде). Там и двусторонний поток данных нужно решить; и, соответственно, вложенность MVC компонента в MVC компонент (нежели, чем просто View).

[alagunoff]

Так, Антон, давай сначала приведем наши взаимоотношения в порядок. Не нужно писать 'ты не правильно понял', если у тебя отличное мнение от моего, нужно так и писать, что мол а я считаю так и так, и мы будем уже обсуждать. Во-вторых, от тебя не требуют больше чем от других стажеров, поэтому, я не буду тратить время на то, чтобы тебя уговаривать что-то сделать) Если не ты считаешь, что к тебе проявляется предвзятое отношение или требуется что-то не посильное, то пиши тогда Сергею и обсуждай с ним эти моменты) Что касается пунктов, то

2. Да, такое может быть. Реализуй такое же поведение слайдера(чтобы, не просто можно было добавить 3 и более ползунков, как у тебя сейчас. Сейчас непонятен их смысл), раз уж ты взялся за это и вопрос будет снят)
3. Не достаточно обоснование за эту точку зрения. Я и другие люди, которые писали слайдер, считаем иначе )

[alanreidt]

@madnessJs − Я когда уверен в том, что говорю, так это и сообщаю. Ты и правда не понимаешь, что такое шаг.

UPD: Посмотрел noUiSlider & Ion.RangeSlider реализации логики шага. У них тоже она отсутствует. Но, я все же остаюсь при своем. Из моих наблюдений за применением слайдера, везде используются равные промежутки шага. Пример из статьи Designing The Perfect Slider:

А вот такого поведения, на практике нигде не встретишь (если только по ошибке):

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

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

UPD: Поговорил с Сергеем. Дополнительную функциональность я реализую. Она отсутствует в требованиях, так как ни у кого нет времени ее туда добавить.

− Нынешняя реализация никак не граничит с требованиями. Она работает с одним и двумя значениями корректно. Добавление большего количества бегунков лишь дополнение. Так что мешает оставить эту функциональность как есть?

[alagunoff]

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

[alanreidt]

@madnessJs

1. А можешь, пожалуйста, обосновать, чем тебя не устраивает мое решение? Вот допустим, есть у нас гостиничные номера с ценой от 1833 ₽ до 17537 ₽ за сутки. А у тебя слайдер с каким-то большим шагом стоит (отличным от 1-цы), чтобы пользователю проще было диапазон цен выбирать. И как ты сделаешь шаг, либо значения на шкале равномерными? Тебе придется считать его программно. А это как раз логика относящаяся к слайдеру.

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

[alanreidt]

Progress:

• [x] Realize Scale component
• [x] Realize additional theme
• [x] Realize Connector component

[alagunoff]

@madnessJs

1. А можешь, пожалуйста, обосновать, чем тебя не устраивает мое решение? Вот допустим, есть у нас гостиничные номера с ценой от 1833 ₽ до 17537 ₽ за сутки. А у тебя слайдер с каким-то большим шагом стоит (отличным от 1-цы), чтобы пользователю проще было диапазон цен выбирать. И как ты сделаешь шаг, либо значения на шкале равномерными? Тебе придется считать его программно. А это как раз логика относящаяся к слайдеру.
2. Не могу понять тебя. У меня реализована возможность задачи нескольких значений для слайдера. Высока вероятность того, что она может пригодится в каком-либо случае — следовательно, раз уж реализована, то смысла удалять нет. Но нет другого функционала, на который у меня нет времени. Пользователь может добавить какие-либо тултупы и т.п. по своему желанию. Так смысл мне сейчас с этим возиться?

1. А зачем тебе делать равномерным значения на шкале? Если у тебя мин=1 макс=10 и шаг=3, то это полностью валидные значения и никаких корректировок шага, как у тебя, быть не должно. Даже после того как ты посмотрел на слайдеры, которыми пользуются тысячи разработчиков, продолжаешь стоять на своем) Это хорошо конечно, но подумай глобальнее...ты топчишься на месте со своей установкой 'не буду переделывать' и не сдвинишься с него пока не начнешь переделывать)
2. Если у тебя нет времени на доделывание того, что начал, то зачем было начинать?)

[alanreidt]

@madnessJs

1. У меня как раз все работает как ты и говоришь, в рамках этого примера: 3 — валидное значение.

А почему в дизайне соблюдаются равные отступы между одинаковыми элементами? Например, как в сетке на мекете Toxin:

Потому что людьми симметрия воспринимается как нечто красивое, а ассиметрия — наоборот,

Или ты думаешь лучше будет сделать как на твоем слайдере? (проставить разных промежутков)

И дело не в том, что мне хочется постоять на своем, а в том как сделать правильнее. А для этого нужно обсудить плюсы и минусы того или иного подхода (и про обсуждение ты сам говорил ранее). Однако, не вижу с твоей стороны никаких попыток, направленных в эту сторону. Больше похоже на то, что ты встал на своем и не собираешься менять свое мнение.

2. Речь не об этом. Основной функционал реализован, а дополнение остается дополнением и никак ему не противоречит. А ты от меня требуешь реализовать какую-то сугубо твою прихоть. А таким образом мы можем долго этим заниматься, так как ты найдешь еще что-либо неподходящее под твой вкус. Только вопрос в том, входит ли это в задачи проверяющего?

[alagunoff]

1. Конечно я думаю будет лучше сделать как у меня, я уже несколько раз об этом сказал) Промежутки и должны быть разные, это нормально, ведь расстояние от 45 до 90 больше чем от 90 до 100. Обсуждение наше зашло в тупик, как сам видишь. Твои доводы о красоте и симметричности не поменяли мое мнение)
2. Хорошо, согласен, вопрос снят)

[alanreidt]

@madnessJs А то, что у тебя шаг разный получается тебя не смущает? 45 — 45 — 10. Да и на реальных проектах ты где-нибудь видел такое применение? Можешь статью просмотреть: https://www.smashingmagazine.com/2017/07/designing-perfect-slider/ и убедиться.

[alagunoff]

Я видел статейку) Т.е. у тебя получается логика: между мин и макс значением располагаются значение на равноудаленном друг от друга расстоянии и значение шага авт. подстраивается, чтобы удовлетворять этим требованиям?

[alanreidt]

@madnessJs Да, грубо говоря. Если более точно, то интервал (разность между макс и мин значениями) должен делиться на значение шага без остатка. Иначе, шаг будет скорректирован к ближайшему значению, удовлетворяющему данному условию.

[alagunoff]

Странная область для нажатия на стрелку

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

Ты можешь разрулить этот момент тем, что будет добавлять/удалять в верстку инпуты по мере увеличение/уменьшения кол-ва ползунков. По стандартам нужно использовать подходящий type у input

Тут у тебя 2 интерактивных элемента по сути(тогл и текст), но по факту у тебя он один и он по середине))

'Slider plugin' не заказчик сайта, хотя тут спорно что это сайт) Подумай над исправлением этого текста

Объясни плиз эту строчку

Connector не подходящее название, обычно этот компонент называется progress bar. Подумай над названием

Включить вертикально?

Мм?

Когда перетягиваю за левый край ползунка, все окей. Когда за правый, как-то прыгает в сторону

Тут наоборот) справа норм, слева если хватать, то скачет ползунок

Невозможно захватит ползунок при таких настройках

[alanreidt]

'Slider plugin' не заказчик сайта, хотя тут спорно что это сайт) Подумай над исправлением этого текста

Это копирайт, который указывает, что данный сайт выполнен для конкретного плагина.

Connector не подходящее название, обычно этот компонент называется progress bar. Подумай над названием

Это зависит от контекста. Progress bar ведь отображает прогресс по какому-либо процессу. Если слайдер для отображения загрузки чего-либо используется, либо времени видео дорожки, то название подходит. Но, если он используется для выбора диапазона цен, то эта полоса уже явно не отображение прогресса. А Connector как-то более в этом случае универсален.

Включить вертикально?

Turn Vertically переводится как "повернуть вертикально".

Это просто заглушка, вместо названия слайдера.

[alagunoff]

Это копирайт, который указывает, что данный сайт выполнен для конкретного плагина.

Мне кажется лучше было бы написать for demonstrating Slider plugin, что-то в этом духе. Тк смысл текущего текста, что сайт выполнен для кого-то, это не совсем правильно

Это просто заглушка, вместо названия слайдера.

Ну у тебя есть название плагина уже, хоть может и не окончательное, но все же. Почему не написать его? Я про Slider plugin. В readme ты его так называешь

[alanreidt]

Исправил.

• По комментарию выше: И в нынешнем состоянии все хорошо, не вижу с этим проблем. В любом случае, эти моменты явно выходят за рамки ревью — я же не контент менеджер.
• По полю “Initial values”: Улучшил удобство для мобильных устройств добавлением numeric inputmode аттрибута. Саму концепцию оставил как есть, так как вариант с добавлением полей только усложнит процесс добавления значений — придется лишний раз указывать количество ползунков и как-то его еще синхронизировать с полями значений, либо постоянно жать кнопку добавить и после этого еще вводить значение.

[alagunoff]

Саму концепцию оставил как есть, так как вариант с добавлением полей только усложнит процесс добавления значений

Я не заставляю делать именно так как я сказал, я лишь сказал, что по стандартам у input должен быть подходящий type, а как это реализовать это уже дело за тобой)

Зачем pointer, если по середине нельзя нажать?

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

Дело не в этом. Т.к. ты нацелен на позицию фронтенд разработчика, то я думаю ты должен осознавать, что ты делаешь строя интерфейсы, и делать это правильно. Этот текст - отсебятина. Если уж ты решил что-то сделать от себя, то делай это правильно)

Ну у тебя есть название плагина уже, хоть может и не окончательное, но все же. Почему не написать его? Я про Slider plugin. В readme ты его так называешь

не исправил

Когда перетягиваю за левый край ползунка, все окей. Когда за правый, как-то прыгает в сторону

не исправил

Тут наоборот) справа норм, слева если хватать, то скачет ползунок

не исправил

[alanreidt]

Я не заставляю делать именно так как я сказал, я лишь сказал, что по стандартам у input должен быть подходящий type, а как это реализовать это уже дело за тобой)

Тут только 2 возможных варианта. Мною выбранный — наиболее удобный. И type="text" наиболее подходящий в данном случае.

Дело не в этом. Т.к. ты нацелен на позицию фронтенд разработчика, то я думаю ты должен осознавать, что ты делаешь строя интерфейсы, и делать это правильно. Этот текст - отсебятина. Если уж ты решил что-то сделать от себя, то делай это правильно)

Так дело в том, что я считаю данный текст правильным. Из него сложно понять что-то другое, нежели чем его истинное значение. И ранее сказанным, я имею в виду, что в рамках данного ревью, у нас нет возможности оценить этот момент объективно — в стандартах нет требований к содержанию.

Ну у тебя есть название плагина уже, хоть может и не окончательное, но все же. Почему не написать его? Я про Slider plugin. В readme ты его так называешь

Ты мне предлагаешь заменить одну заглушку на другую.

Когда перетягиваю за левый край ползунка, все окей. Когда за правый, как-то прыгает в сторону

Тут наоборот) справа норм, слева если хватать, то скачет ползунок

У меня все нормально. Курсор смещается к центру, но это вполне адекватное поведение.

[alagunoff]

Мною выбранный — наиболее удобный

Удобный для тебя, это да, но не подходящий под стандарты компании.

Так дело в том, что я считаю данный текст правильным. Из него сложно понять что-то другое, нежели чем его истинное значение. И ранее сказанным, я имею в виду, что в рамках данного ревью, у нас нет возможности оценить этот момент объективно — в стандартах нет требований к содержанию.

Ну вот я к примеру понимаю его по-другому. В стандартах нет этих требований, верно, да и это вообще не требование. Опять же таки, если ты что-то делаешь от себя, то объем моей работы возрастает прямо пропорционально кол-ву дополнительных вещей, которые ты добавляешь, т.к. необходимо проверить как ты понимаешь ту или иную вещь. Если ты будешь работать и пилить непонятный код, который у тебя будут также ребята проверять(даже после устройста), то ты вместе исправлений будешь говорить, что ты считаешь это правильным, то это не дело)) Я не говорю, что мой вариант единственно верный, я говорю что твой вариант несет другой смысл. Если это не так, то можешь объяснить мне, вдруг я не правильно понимаю.

Ты мне предлагаешь заменить одну заглушку на другую

Нет) Я предлагаю написать нормальное имя плагина. Опять же таки, не настаиваю. Если это не окончательное название для плагина, то можешь так и написать мне)

У меня все нормально. Курсор смещается к центру, но это вполне адекватное поведение.

Ну я же тоже не на чьем-то слайдере пробовал, а на твоем) Это не совсем адекватное решение. По твоему если, к примеру, мы берем палку с земли, что кинуть ее, к примеру, она должна в руке перескакивать на середину?

[alanreidt]

Удобный для тебя, это да, но не подходящий под стандарты компании.

Почему не подходящий? В стандартах написано:

21. Если у поля есть четкое назначение, то использовать соответствующий тип (email, number, color и тд);

То есть, нужно использовать тот тип поля, «браузерная» реализация которого уже решает часть требуемых задач. В моем случае, только поле с типом text является таковым. Поле с типом number только усложнит UX: 1) Либо получится вариант, где пользователю придется продумывать заранее количество ползунков, корректировать его, в случае ошибки; 2) Либо, постоянно жать кнопку добавить и после этого еще вводить значение.

[alanreidt]

Ну я же тоже не на чьем-то слайдере пробовал, а на твоем) Это не совсем адекватное решение. По твоему если, к примеру, мы берем палку с земли, что кинуть ее, к примеру, она должна в руке перескакивать на середину?

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

[alagunoff]

То есть, нужно использовать тот тип поля, «браузерная» реализация которого уже решает часть требуемых задач. В моем случае, только поле с типом text является таковым.

Согласен, оставляем.

По контентному содержимому тоже все вопросы сняты.

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

Проблемы ui, определенно. Слайдер может работать и без многих других вещей, только вопрос в том, насколько хорошо? Окей, дело твое, я тебе указываю на ошибки, а твое дело их исправить или нет)

Перекрываются подсказки при наложении друг на друга

Невозможно схватить ползунок при таких настройках

Странное поведение при таких настройках

[alanreidt]

Странное поведение при таких настройках

Почему же странное? Никто не станет вводить данные значения намерено, только при ошибке. А слайдер эту ошибку покажет.

[alagunoff]

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

По этому вопросу, уточнил у команды. У всех однозначное мнение, что нужно переделать.

Почему же странное? Никто не станет вводить данные значения намерено, только при ошибке. А слайдер эту ошибку покажет.

Как советуют ребята в react, если есть какая-то ошибка, то приложение вообще не должно собираться. У тебя сейчас собирается, но криво. Поэтому, лучше выкидывать ошибку при таких настройках, или валидировать этот момент.