Начав работу на прошлой неделе, я, наконец, создал PR, который частично рефакторил файл move.js (было около +60, -60 изменений). Я внес изменения в следующем формате (например):
Изменено с document.getElementById(‘selector’).style.display = ‘none’; на $(‘#selector’)[0].style.display = ‘none’;
т. е. везде изменен document.getElementById(‘selector’) на $(‘#selector’)[0].
В целях тестирования я добавил пример, чтобы показать, что такого рода изменения не подведут и будут работать. Вот пример, упомянутый в PR:
Использование document.getElementById(‘selector’)
После создания этого PR Девен Бансод прокомментировал:
«это будет работать только для элементов, выбранных с идентификаторами. В идеале я бы использовал метод .css() для установки стилей вместо ручного выбора первого элемента и установки собственных свойств. IMO (даже несмотря на то, что он работает), этот текущий подход вообще не использует возможности JQuery.
Использование .css() автоматически зациклится на всех выбранных элементах, поэтому его будет легче читать, когда мы выбираем на основе типы ввода, классы и т. д.»
Кроме того, один из наставников, Сакшам Гупта, поддержал этот комментарий и добавил: «Не пытайтесь жестко кодировать вещи. Когда вы пытаетесь реорганизовать код, постарайтесь сделать это таким образом, чтобы другие разработчики могли работать над кодом без особых изменений в вашем коде. Становится чрезвычайно трудно управлять такими вещами в таком большом масштабе».
С этими комментариями я, наконец, получил хорошую картину и начал работать над этим. Таким образом, я снова начал менять вещи, вот тип изменений, которые я сделал:
- От "document.getElementById('selector').style.display = 'none';"
до "$('#selector').css( «отображать», «нет»); - От "document.getElementById('selector').style.position = 'absolute';"
до "$('#selector').css( «позиция», «абсолютная»); - От "document.getElementById('selector').style.zIndex = '103';"
до "$('#selector').css( «z-индекс», «103»); - От "document.getElementById('selector').style.visibility = 'visible';"
до "$('#selector').css( «видимость», «видимый»); - От "document.getElementById('selector').style.left;"
до "$('#selector').offset().left; ” - От "document.getElementById('selector').style.top;"
до "$('#selector').offset().top; ” - От "document.getElementById('selector').style.left = value + 'px';"
до "$('#selector'). смещение({слева: значение});” - От "document.getElementById('selector').style.top = value + 'px';"
до "$('#selector'). смещение({верхнее: значение});” - От «document.getElementById(‘selector’).width = value;»
до «$(‘#selector’).width(value);» - От «document.getElementById(‘selector’).height = value;»
до «$(‘#selector’).height(value);» - От "document.getElementById('selector').style.width;"
до "$('#selector').width();" - От «document.getElementById(‘selector’).style.height;»
до «$(‘#selector’).height();» - От "document.getElementById('selector').offsetWidth;"
до "Math.round(parseFloat($('selector').outerWidth()) ))” - От "document.getElementById('selector').offsetHeight;"
до "Math.round(parseFloat($('selector').outerHeight()) ))” - От "document.getElementById('selector').checked"
до "$('selector).is(":checked")" - От "document.getElementById('selector').checked = false;"
до "$('selector).prop('checked', false) ;” - От "document.getElementById('selector').offsetLeft"
до "$('selector').position().left + parseInt($( 'селектор').css("marginLeft"))” - От "document.getElementById('selector').offsetTop"
до "$('selector').position().top + parseInt($( 'селектор').css("marginTop"))” - От "document.getElementById('selector').innerHTML === 'v'"
до "$('selector).html() == = 'в';” - От «document.getElementById(‘selector’).innerHTML = value»
до «$(‘selector’).html(value);» - От "document.getElementById('selector').className = value;"
to "$('selector).addClass(value);" - От "document.getElementById('selector').value"
до "$('selector).val()" - От «document.getElementById(‘selector’).className === value»
до «$(‘selector’).hasClass(value)»
Я протестировал все это на w3school. Для тестирования я открыл что-то в стандартном ванильном JavaScript с объектами DOM в их браузере Tryit, внес изменения в соответствии с jQuery и запустил то же самое. Таким образом, не было никаких изменений в результатах до и после внесения изменений, что подтверждает правильность синтаксиса. Вот пример для одной из вышеперечисленных ситуаций:
Здесь поиск подходящего синтаксиса и его проверка заняли у меня много времени. И, наконец, после всего этого я зафиксировал изменения без жестко запрограммированных вещей.
Кроме того, Девен Бансод прокомментировал и указал на те же проблемы с предыдущим PR, которые были частью работы фазы 1. Однажды, когда я закончил частичный рефакторинг move.js, я сделал и разрешил то же самое для этих файлов, как указал Девен:
- https://github.com/phpmyadmin/phpmyadmin/pull/ 15339
— https://github.com/phpmyadmin/phpmyadmin/pull/15338
Так что теперь буду ждать отзывов от наставников. После внесения всех запрошенных изменений нам нужно повторить то же самое для остальной части файла move.js и файла history.js. Кроме того, необходимо завершить работу с документацией на седьмой неделе GSoC.
