Conversation
flash42
left a comment
flash42
left a comment
There was a problem hiding this comment.
I left a couple of comments inline. The only structure problem I could see (was not checking user facing things) is the one regarding the way you put half-baked elements in the dom and later make lookups to add event listeners to them.
| SELECT id, board_id, title, status_id, card_order FROM card; | ||
| """) | ||
|
|
||
| cards = cursor.fetchall() |
|
|
||
| def get_boards(force=False): | ||
| return _get_data('boards', BOARDS_FILE, force) | ||
| cards = cursor.fetchall() |
| """) | ||
|
|
||
| cards = cursor.fetchall() | ||
| print(cards) |
| getCardsByBoardId: function (boardId, callback) { | ||
| // the cards are retrieved and then the callback function is called with the cards | ||
|
|
||
| this._api_get(`/get-cards/${boardId}`, (response) => { |
There was a problem hiding this comment.
(response) => {} => response => {} // No need for () for a single parameter lambda.
| boardContainer.textContent = ''; | ||
| for (const board of boards) { | ||
| const template = document.querySelector('#board-template'); | ||
| const clone = document.importNode(template.content, true); |
There was a problem hiding this comment.
What is clone - doesn't tell me what it is. Maybe newBoard, or something it really is.
static/js/dom.js
Outdated
| ${boardList} | ||
| </ul> | ||
| `; | ||
| for (let card of cards) { |
There was a problem hiding this comment.
Sometimes there is const, here it is let in the for-each loop. Please use it consistently. Const is preferred over let, if you don't want (here you don't) to overwrite it.
| } | ||
| }, | ||
| addNewBoard: function () { | ||
| let addBoardButton = document.querySelector("#add-new-board-btn"); |
| }) | ||
| }, | ||
| showNewBoard: function (response) { | ||
| let boardContainer = document.querySelector('.board-container'); |
| loadCards: function (boardId) { | ||
| // retrieves cards and makes showCards called | ||
| addNewCard: function (boardId) { | ||
| let addNewCardButtons = document.querySelectorAll(".board-add"); |
There was a problem hiding this comment.
o_O This is hard to follow form the code. Why we need to look up these addNewCardButtons? More interesting question is why do we have board-add class on them? Can we not create the button with the proper event listener before we add it to the dom?
There was a problem hiding this comment.
Ask if something is not clear :)
|
|
||
| if (parseInt(response.board_id) === parseInt(column.dataset.boardId) && | ||
| parseInt(response.status_id) === parseInt(column.id)) { | ||
| const template = document.querySelector('#cards-template'); |
There was a problem hiding this comment.
There is a lot of string literals scattered across the code. Can you please extract them to methods?
e.g.:
const template = document.querySelector('#cards-template') =>
getCardsTemplate(): function() {
return document.querySelector('#cards-template').content;
}
const newCard = document.importNode(getCardsTemplate(), true);
…ing to input field with original title is ok
…. but with DOM manip., it works perfectly
…board, it gets displayed immediately at the end of the bpards instead of staying where it is. however, renamed board's new name appears also immediately.
5 participants
No description provided.