this is my site#5
Conversation
| <html lang="en"> | ||
| <head> | ||
| <link rel="stylesheet" href="styles.css"> | ||
| <!--import icon font--> |
There was a problem hiding this comment.
where did u take the template of index.html from?
| <div style='display:flex;align-self: flex-start; margin:5px'>description: <input id='description-input' class='description-input' style='height:50px' type='textarea'/></div> | ||
| </div> | ||
| <div class='menu-buttons'> | ||
| <!-- <button class='clear-button' onclick='clear_menu()'>clear</button> |
| @@ -0,0 +1,170 @@ | |||
| var edited_id = 'empty'; | |||
There was a problem hiding this comment.
we don't use var anymore. use either const or let.
const for pointers/primitive that won't change.
let for pointers/primitive that you will need to change their values
| <div class='item'> | ||
| <div style='margin:5px'>title: <input id='title-input' required class='title-input' type='text'/></div> | ||
| <div style='margin:5px'>subtitle: <input id='subtitle-input' class='subtitle-input' type='text'/></div> | ||
| <div style='display:flex;align-self: flex-start; margin:5px'>description: <input id='description-input' class='description-input' style='height:50px' type='textarea'/></div> |
There was a problem hiding this comment.
- put all of your css rules inside
cssfiles and not inline. - make description a multiline text input aka
textareaso I will be able to write a long todo (more than 1 long line)
There was a problem hiding this comment.
did u fix this? (second bullet point)
| <ul id='list' class='todo-list'> | ||
| <!-- to be added dynamically--> | ||
| </ul> | ||
| <button class='add-button' onclick="add_button(this)">add new todo item</button> |
There was a problem hiding this comment.
make it so hitting enter will also add a new todo
| function add_item(){ | ||
| let title = document.getElementById('title-input').value | ||
| if (title ==''){ | ||
| console.log('cant add an item without a title') |
There was a problem hiding this comment.
the console is not user facing, i.e a normal user will never see these messages. use alert instead.
In the future we'll improve it and we will use different mechanism for alerting the user that he did something wrong. for now alert is good enough
| document.getElementById('menu-button').innerText = 'add item' | ||
|
|
||
| // dont forget to change the id variable | ||
| id++ |
There was a problem hiding this comment.
an incrementing id param is usually not a good idea. it might work for now but once we'll introduce server or even local storage, it will become a sync nightmare. instead we use GUID for ids.
read more about it here:
https://dba.stackexchange.com/questions/264/guid-vs-int-which-is-better-as-a-primary-key
| let itemTextElement = document.createElement('div') | ||
| itemTextElement.className = 'todo-item-text' | ||
| // item title | ||
| let titleElement = document.createElement('div') |
There was a problem hiding this comment.
i don't understand the indentation here.
| itemElement.appendChild(itemTextElement) | ||
| itemElement.appendChild(itemButtonsElement) | ||
|
|
||
| // the itemElement should look like this example: |
There was a problem hiding this comment.
we don't use comments in our code. instead the code needs to tell the story in the most clear and easy to understand way possible.
in the words of uncle bob:
"A comment is a failure to express yourself in code. If you fail, then write a comment; but try not to fail"
| list.appendChild(itemElement) | ||
|
|
||
| // reset menu display | ||
| document.getElementById('title-input').value = '' |
There was a problem hiding this comment.
document.getElementById(X).value is something that you have a lot of.. think of a way to shorten it for example
setElementValue(x) or even just setValue(X)
… alerts, accessive indentaions removed
2 participants
a basic vanilla TODO list app