Прокрутка к текущей категории не работает начиная с версии 2.8.0.beta5

Внести вклад Ошибка
1

В версиях Discourse до 2.8.0.beta5, если вы редактируете тему и нажимаете на раскрывающийся список выбора категории, текущая категория одновременно выделяется и прокручивается в списке категорий так, чтобы она сразу была видна вверху, а её подкатегории (если есть) также отображались. Это позволяло легко переместить тему в более подходящую подкатегорию.

Работа в версиях 2.8.0.beta4 и ранее:

Начиная с версии 2.8.0.beta5, при открытии раскрывающегося списка выбора категории прокрутка к текущей категории больше не выполняется:

Надеюсь, эти короткие видео-демонстрации помогут быстро показать то, что я пытаюсь описать.

Примечание: в моих примерах включены опции «фиксированные позиции категорий» и «фиксированные позиции категорий при создании», однако поведение прокрутки к текущей категории отсутствует независимо от того, включены эти опции или выключены.

Я подтвердил это в локальной разработке, установив версию 2.8.0.beta4 и наблюдая желаемое поведение, а затем обновившись до версии 2.8.0.beta5, где это поведение перестало работать.

Я изучил код, пока не нашёл, что изменение произошло в большом коммите, связанном с select-kit.js (подождите немного, и страница должна перейти к соответствующим строкам кода):

Из метода _scrollToRow(), который вызывается методом _scrollToCurrent(), был удалён следующий код:

if (rowContainer) {
  const collectionContainer = rowContainer.parentNode;

  collectionContainer.scrollTop =
    rowContainer.offsetTop - collectionContainer.offsetTop;
}

Я подтвердил, что добавление этого кода обратно в select-kit.js в версии 2.8.0.beta5 решает проблему, однако, поскольку мне неясно, почему он был удалён, я не уверен в возможных побочных эффектах его возврата.

Спасибо за внимание к этой проблеме, и надеюсь, что мы сможем восстановить это поведение в одном из будущих релизов.

4 лайка
3

Привет, спасибо за это :+1:

Я точно не помню, почему я это сделал, но насколько я могу судить сейчас, думаю, это тоже ошибка.

Я создал этот PR:

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

4

Привет, Жоффри, спасибо, что уделил время этому! Я протестировал последние изменения, но, похоже, они не сработали. Поскольку изменения уже были слиты, я проверил на ветке tests-passed.

Из двух коммитов в PR первый работает (ab0fbf1), но переопределяется вторым (92943ff).

Изменение на строке 992 выглядит корректным, но не даёт ожидаемого эффекта. Похоже, что необходим код, который был удалён и который корректирует значение collectionContainer.scrollTop.

Спасибо ещё раз!

1 лайк
5

Да, я не был уверен, что этого поведения будет достаточно. Я могу добавить это обратно завтра. Не могли бы вы более точно объяснить, что именно вам не нравится в этом поведении сейчас, пожалуйста?

6

Привет, Жоффрей,

Спасибо за продолжение работы над этим вопросом. При текущих изменениях в коде (я убедился, что загрузил последнюю версию, очистил папку tmp и перезапустил сервер разработки) поведение, которое было в версиях до 2.8.0.beta5, не восстановилось. Список по-прежнему не прокручивается так, чтобы текущая категория отображалась в верхней части видимых вариантов в выпадающем меню.

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

Строка rowContainer?.focus({ preventScroll }); в данном случае, по-видимому, не оказывает никакого эффекта, независимо от того, установлено ли значение preventScroll в true, false или даже если вся строка удалена.

Ключевая строка, которая обеспечивает правильное поведение (отсутствует в текущем релизе), выглядит так:

collectionContainer.scrollTop =
          rowContainer.offsetTop - collectionContainer.offsetTop;

Без неё прокрутка к нужной позиции не происходит, так как scrollTop по умолчанию равен 0. После выполнения этой строки scrollTop принимает правильное значение в пикселях, необходимое для прокрутки вниз.

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

Спасибо!

7

Да, он не в самом верху, но находится в области просмотра, чего, на мой взгляд, достаточно.

8

Привет, Жоффри,

Спасибо за обратную связь. Я добавил больше категорий в свой демо-пример и подтвердил, что текущая категория теперь отображается в области просмотра — это определённо улучшение. Тем не менее, было бы идеально, если бы она снова отображалась вверху списка, как раньше. Это обеспечило бы немедленную видимость большего количества подкатегорий под ней, и пользователь мог бы перейти к подкатегории, не прокручивая страницу вниз, чтобы увидеть их. На мой взгляд, это более весомый сценарий использования, чем отображение соседних несвязанных категорий. Изменение на центрирование было сделано намеренно или это случайный регресс предыдущего поведения? В любом случае, решение за вами — просто делюсь своим мнением.

Ещё раз спасибо, и я ценю вашу тяжелую работу над Discourse!