Uh oh!
There was an error while loading. Please reload this page.
Callbook - #20
Conversation
OlegLustenko
left a comment
There was a problem hiding this comment.
please move bootstrap to cdn and remove scss from git
| </form> | ||
| <table class="table table-hover contacts"> | ||
| ${createTh(['Name',"Last name", "Number"])} |
There was a problem hiding this comment.
please move function methods to the class
| ${createNav("glyphicon glyphicon-search", ``,`true`)} | ||
| ${createNav("tab-text", `Context`)} | ||
| </a> | ||
| <a href="keypad.html" class="tab"> |
There was a problem hiding this comment.
I believe you could make links more reusable.
What if one day you have to set class but the click ? or something more expensive.
I guess it could look that way
<navclass="main-nav">
this.renderLink({content:'Contacts',className:'active',icon:'search'});
this.renderLink({content:'Keypad',icon:'th'});
this.renderLink({content:'Edit contact'});
...
</nav>We could reuse some HTML parts and create a smaller reusable blocks
| let buildHtmlTags = () => { | ||
| let body = document.body; | ||
| body.innerHTML += `<header class="header"> |
There was a problem hiding this comment.
document.body.innerHTML not sure I like to use document.body.
You have to create an additional tag at index.html and call it, for example,
<divid="mountNode"></div>And update content only inside such node. updating the whole document.body is too risky
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script> | ||
| <script src="js/bootstrap.min.js"></script> |
There was a problem hiding this comment.
please remove bootstrap and jquery
| numberAdd(arr){ | ||
| let numbArr = arr.map( value => | ||
| `<button class="key" OnClick=" myKeyad.outputNumb(${value}) ">${value}</button>` |
There was a problem hiding this comment.
are you sure about OnClick handler ?
| <span id="numberInput" class ="numbers"></span> | ||
| <span class="glyphicon glyphicon-circle-arrow-left" aria-hidden="true" OnClick="myKeyad.glyphicon()"></span> | ||
| </div> | ||
| ${this.numberAdd(['1','2','3','4','5','6','7','8','9'," \* " ,'0',"\#" ])} |
| main.innerHTML += `<header class="header"> | ||
| <div class="container top-radius"> | ||
| <div class="user-top-line"> | ||
| <a href="index.html"> |
No description provided.