Conversation
Week-2/style/style.css
Outdated
| padding:10px; | ||
| } | ||
| .closeButton{ | ||
| } |
Week-2/scripts/RenderContent.js
Outdated
| let data=[]; | ||
| for(item in article_data){ | ||
| if(data.indexOf(article_data[item].postCategory)!==-1){ | ||
| continue; |
There was a problem hiding this comment.
generally if else block is indented as below example
if(condition) {
//code
} else if(condition) {
//code
} else {
//code
}
Week-2/scripts/RenderContent.js
Outdated
| fullContainer.appendChild(right_container); | ||
| select.options[0].disabled = true; | ||
| select.onchange = () => { | ||
| let val=select.value; |
There was a problem hiding this comment.
can get rid of this line and use the expression in below line.
| let val=select.value; | ||
| changeContent(val,container); | ||
| } | ||
| } |
There was a problem hiding this comment.
consider splitting this function to few smaller functions
Week-2/scripts/constants.js
Outdated
| @@ -0,0 +1,89 @@ | |||
| const defaultContent='Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'; | |||
| const defaultDescription='created at 21 July 2019""created at 21 July 2019'; | |||
There was a problem hiding this comment.
can use single const declaration for all variables.
| card.appendChild(right); | ||
| container.appendChild(card); | ||
| container.appendChild(horizontalline); | ||
| fullContainer.appendChild(container); |
There was a problem hiding this comment.
consider splitting this function into few smaller functions
Week-2/scripts/utility.js
Outdated
| if(regexEmail.test(emailid)){ | ||
| window.localStorage.setItem(emailid, JSON.stringify(emailid)); | ||
| alert(emailSubscribedMessage); | ||
| } |
There was a problem hiding this comment.
generally if else block is indented as below example
if(condition) {
//code
} else if(condition) {
//code
} else {
//code
}
Week-2/scripts/utility.js
Outdated
| popup.appendChild(descriptionPopup); | ||
| fullContainer.style.opacity = '0.3'; | ||
| } | ||
| else if(clickEvent.srcElement.id === 'closePopup'){ |
There was a problem hiding this comment.
generally if else block is indented as below example
if(condition) {
//code
} else if(condition) {
//code
} else {
//code
}
| right_container.appendChild(titleSelectCategory); | ||
| right_container.appendChild(select); | ||
| right_container.appendChild(subscribeTitle); | ||
| right_container.appendChild(check); |
There was a problem hiding this comment.
few places same dom manipulation function used, this can be clubbed by making a few common functions. this can reduce a lot of code lines.
discuss with me if you are not clear what i am telling
-> using a common createCard function for both static and news data,to reduce dom code in a single function ->other minor changes
|
looks good |
3 participants
No description provided.