Templates, webpack, Emitter, LocalStorage #4
Conversation
| "dependencies": { | ||
| "todomvc-app-css": "^2.0.0", | ||
| "todomvc-common": "^1.0.0", | ||
| "watchify": "^3.7.0", | ||
| "browserify": "^13.1.1" | ||
| "browserify": "^13.1.1", |
There was a problem hiding this comment.
А зачем оставил browserify, если борка переведена на webpack?)
|
Добавь плз инструкцию как запускать твое приложение |
|
Ты сам стал реализовывать класс https://nodejs.org/api/events.html#events_class_eventemitter ? Здорово! |
|
templatesStorage - в принципе норм шаблонизатор, единственное что я бы хранил не функцию шаблонизации, а сам шаблон. У тебя функция же одинаковая будет, а не то что один шаблон на underscore написан, другой на handlebars |
|
Не увидел у тебя ни модели, ни контроллера. Из вью - только шаблонизатор |
|
Была техническая проблема, поэтому пока почти ничего не работает) То есть работает роутер, но этого не видно. Сам MVC (почти, осталось View и немного Controller) написан: EventEmitter, TemplatesStorage, Model, Router+Url, Controller, View. Есть местами документация. Сделаны заготовки для самих вьюх, контроллеров и прочего относящегося к приложению, а не фреймворку. Если успею, за понеделиник что-то уже должно заработать. |
|
Всё готово. |
|
Начал проверять, обновил плз readme и добавиь инструкцию как запускать твое приложение. Добавь информации в package.json и скрипты туда тоже добавь, чтобы можно было легко запускать |
|
eslint ты так и не настроил? попробуй настроить любой другой линтер, ну и было бы здорово, если ты с ним разобрался, как его настроить |
|
Ты так в итоге не доделал роутинг, чтобы между состояниями |
|
Очень сложная у тебя получилась модель, попробуй подробнее расписать для чего каждый метод нужен и какой стандартный порядок работы с моделью. У тебя много статических свойств, которые используют this. Статические свойства отличаются тем, что им не нужен this иначе их стоит положить в прототип |
|
Ещё у тебя постоянное смешение es6 и es5 классов, лучше писать в одной парадигме |
|
По поводу взаимодействия вьюх и моделей. Представь, что у тебя в список могут быть добавлены модели из разных мест, тогда вьюха у тебя не узнает, что нужно добавить ещё одну тудушку |
|
Попробуй вынести футер в каждом тудулисте, где completed, uncompleted в отдельную вьюху, а не всё делать из тодолист вьюхи. Ещё у тебя изменение в большей степени завязано на изменение внутренней вьюхи, а стоит завязываться на изменение модели |
| // firstList.commit(); | ||
| // } | ||
|
|
||
| router.navigate({pathname: pathnamePrefix + '/index.html'}); |
There was a problem hiding this comment.
У тебя же в рамках одного приложения роутер работает, поэтому лучше не по полнопу пути навигироваться, а по относительному, а ещё, лучше навигироваться по именам, а не фактическим путям, т.е. router.navigate('index') - и внутри роутера уже разворачиваить и менять урл как тебе нужно.
Такая запись будет короче и понятнее
There was a problem hiding this comment.
Да, про имена у меня возникала идея, но из-за того, что приложение состоит фактически из 1 страницы, решил это пока отложить.
| let router = new Router([ | ||
| { | ||
| url: index, | ||
| controller: new TodoController() |
There was a problem hiding this comment.
Да, вот в этом месте можно было задать, например:
{
name: 'index',
url: index,
controller: ...
}
И навигироваться уже по полю name
| import {Controller} from 'FKM'; | ||
|
|
||
|
|
||
| class TodoController extends Controller { |
There was a problem hiding this comment.
Хорошо, когда файл назвается так же как и экспортируемый класс
| * @param{String} templateName | ||
| * @param{Object} context | ||
| */ | ||
| render(templateName, context) { |
There was a problem hiding this comment.
А зачем тебе эта функция, если у тебя только .get используется из данного класса)
Задумка этого класса хорошая - покажи его полную мощь, используя в своем приложении
|
|
||
| function View(rootElement, id) { | ||
| (View.__proto__ || Object.getPrototypeOf(View)).call(this); | ||
| this.rootElement = rootElement; |
There was a problem hiding this comment.
Можно было бы сделать немного круче, если в качестве rootElement ты мог бы передавать ещё и строку и вьюха автоматически создавала DOM элемент с тегом переданной строки
There was a problem hiding this comment.
Немножно не понял. Если во вьюху передавать строку, а не элемент, как она поймет куда его в DOM вставлять?
| @@ -0,0 +1,64 @@ | |||
| import {TemplatesStorage} from 'FKM'; | |||
|
|
|||
| function _todoListTemplate(c) { | |||
There was a problem hiding this comment.
Шаблоны можно по разным файлам разнести
| } | ||
|
|
||
| _render() { | ||
| let context = {id: this.id, subviews: {lists: this.subviews.todoLists}}; |
There was a problem hiding this comment.
Когда смотришь в код и видишь this.subviews.todoLists, а в шаблоне видишь this.subviews.lists - то немного теряешься, лучше называть одинаково, чтобы не было путаницы и код легче воспринимался
| addTodoList(todoListObject) { | ||
| let todoListRootElement = document.createElement('section'); | ||
|
|
||
| let todoListView = new TodoListView(todoListRootElement, todoListObject.id); |
There was a problem hiding this comment.
Можно сразу передавать модель, зачем тебе передавать id, а потом во вьюхе снова искать. Так же и с mainView можно сразу передвать коллекцию моделей
| let value = this.ui.inputField.value.trim(); | ||
| if (!value) return; | ||
|
|
||
| let todoObject = this.todoListModel.addTodo(value); |
There was a problem hiding this comment.
Хоть у тебя внутри модель и тригерит событие добавления, ты его нигде не слушаешь
| if (event.type === 'keypress' && event.key !== 'Enter') return; | ||
|
|
||
| let value = this.ui.changeTitleField.value; | ||
| this.todo.title = value; |
There was a problem hiding this comment.
То что у тебя изменился заголовок вьюхи, у тебя никто не узнает, кроме самой этой вьюхи
Реализованы шаблоны. Внешний интерфейс немного отличается от того, который есть в настоящих шаблонизаторах, но без использования сторонних библиотек удалось придумать только в таком виде.
Добавлена сборка с помошью webpack (надеюсь это легально).
Emitter сделан не полностью как в библиотеке, но основные функции есть.
localStorage - небольшая обертка над window.localStorage.
Я не совсем понял что именно нужно реализовывать. Концепцию не понял. То есть пункты которые были в презентации по отдельности сделаны, теперь их как-то, наверное, нужно склеить чтобы был результат.