search

web-standards-ru / wsd.events

Web Standards Days
http://wsd.events
42 stars 29 forks source link

Добавляет сворачивалку календаря на главной (Issue #153) #154

Closed xaosaki closed 6 years ago

xaosaki commented 6 years ago

Сделал. Трюки с location.hash нужны для скромной реализации прокрутки.

pepelsbey commented 6 years ago

Спасибо :) Придрался по мелочам, ну и нужно потестировать и подумать, насколько display: none в такой ситуации мешает доступности и если да — как сделать лучше.

xaosaki commented 6 years ago

display: none в данном случае единственный выход, пробовал с visually-hidden, но там при навигации с клавиатуры фокус уходит в скрытые блоки и на кнопку чтобы попасть табом надо раз 25 клацнуть.

xaosaki commented 6 years ago

@pepelsbey поправил это дело, теперь даже фокус куда нужно уходит. Послушал Сирей, вроде все понятненько. Только текст на кнопке поменял, чтобы было понятно к чему она относится.

tsergeytovarov commented 6 years ago

Привет! Я тоже придрался по мелочам, есть некоторые вещи которые надо поправить. И я по доброму, с лучшими побуждениями!

xaosaki commented 6 years ago

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

tsergeytovarov commented 6 years ago

@xaosaki Привет. Спасибо!

Просто если пулл-реквест описывает одну фитчу, то он делает одну фитчу. А трогать код выше, чтобы перенести комменты, можно сделать в рамках рефакторинга кода отдельной верткой.

По поводу значения по умолчанию, у тебя не стояло у кнопки type='button', а значит что она по умолчанию была submit, а значти что-то у неё было. Как раз на такой случай и для безопасности .preventDefault(). Вдруг мы решим сделать это ссылкой в будущем, по неводомым причинам :)

@pepelsbey у меня всё, если что.

tsergeytovarov commented 6 years ago

@pepelsbey вообще я тут подумал для прогрессивной деградации было бы неплохо сделать так.

  1. Кнопка — это ссылка на страницу, на которой весь развёрнутый календарь, без всяких кнопок показать больше.

  2. Повесить на кнопку ивент, который раскрывал бы их на главной странице.

  3. В случае выключенного JS это кнопка просто переходила бы на страницу с календарём.

Что думаешь?

pepelsbey commented 6 years ago

@tsergeytovarov, у нас весь контент есть на странице. Сворачивалка только делает его компактнее. Смысла в отдельной странице нет никакого. Можно просто прикручивать сворачивалку и всё инициализировать только в случае включённого JS.

pepelsbey commented 6 years ago

Я немного отрефакторил скрипт, чтобы он прятал годы и показывал кнопку только в момент инициализации. Плюс строки хранятся только в HTML и не используются стили в скрипте — только классы. Предыдущий код закомментирован и я бы оттуда впилил фокус и историю.

@xaosaki, доделаешь? :)

xaosaki commented 6 years ago

@pepelsbey с твоей реализацией есть проблемка, я тоже хотел сделать без классов в html изначально, но там получается достаточно стремненькое мигание при плохом интернете, с которым никак не справиться. 👎

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

+ ты забыл обработать ситуацию с количеством эвентов меньше чем лимит и скрытием кнопки, хотя, конечно, это наверное ситуация невозможная и можно её и не обрабатывать.

pepelsbey commented 6 years ago

Ну что, я заинлайнил скрипт сразу после календаря (единственное место, где он используется, вполне оправданно), так что даже на медленном 3G он сразу работает, ещё до шрифтов и всех картинок. Как вам такой вариант? :)

xaosaki commented 6 years ago

@pepelsbey в принципе можно и заинлайнить 👍

Я вернул методы для фокуса и чуть переименовал дата аттрибуты, теперь все по красоте. Что-то еще нужно?