-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
почти всё. добавляет, считывает... #33
base: master
Are you sure you want to change the base?
Conversation
this.container1 = document.getElementById('container'); | ||
this.scroll = document.getElementById('scroll'); | ||
this.arrEditMainInfo = ['First Name', 'Last Name', 'Company']; | ||
this.arrAdd = ['add home phone', 'add email', 'add address', 'add birthday', 'add social profile', 'add field'] | ||
this.arrAdd = ['add fullName', 'add email', 'add birthdate', 'add address', 'add gender '] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вместо массива лучше сделать статические декларативные данные в верстке.
Массив превращает в одном из мест твой код в циклы и ненужными условиями - это не тот сценарий где нужно бояться дублирования кода.
Вот создание каждого инпута - это можно вынести "вроде отдельного компонента"
phoneApp/src/edit_contacts.js
Outdated
console.log(`nnn`, id) | ||
this.bd.forEach(element => { | ||
if (element._id == id) { | ||
this.arrAdd[0] = 'name - ' + element.fullName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
доступ по индексу это плохая практика, этого нужно избегать
phoneApp/src/footer.js
Outdated
</a> | ||
</nav> | ||
<nav class="main-nav" id = "nav1"> | ||
<a href="" class="tab active"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href="keypad.html" class="tab">
<span class="glyphicon glyphicon-th" aria-hidden="true"></span>
<span class="tab-text">Keypad</span>
</a>```
а вот это хороший кандидат чтобы нарисовать ссылку
phoneApp/src/footer.js
Outdated
<span class="glyphicon glyphicon-plus" aria-hidden="true"></span> | ||
<span class="tab-text">Add user</span> | ||
</a> | ||
</nav> | ||
</div>` | ||
const footer = document.getElementById('fo-er') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
этот код нужно сделать классом, назвать роутинг(т.е переходы по страницам) и у него вызываьт методы
phoneApp/src/keypad.js
Outdated
class Keypad { | ||
constructor() { | ||
this.lol = document.getElementById('lol'); | ||
this.arr = ['1', '2', '3', '4', '5', '6', '7', '8', '9', "'*'", '0', "'#'"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
цифры у нас всегда статичны и создавать из них массив нет смысла.
Данные которые приходят с сервера или меняются состояние приложения - необходимо хранить в структурах данных, остальные данные в 95% случаев это статика или декларативный код
// императивный вариант - менее читабельный
const numbers = [1,2,3]
const numbersData = numbers.reduce((allNumbers, number) => allNumber + `<li>number</li>`, ``)
декларативный вариант
const numbersData_v2 = `
<li>1</li>
<li>2</li>
<li>3</li>
`
второй случай очевиднее, нам не нужны калькуляции для этого, потому что у нас никогда не появится цифра "11" например
phoneApp/src/keypad.js
Outdated
this.memory += `${num}`; | ||
if (this.memory === '#' || this.memory === '*') { | ||
this.memory = this.memory; | ||
} else if (this.memory.length == 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это нужно сделать при помощи regExp, можно гуглить, общаться в чате или как-то еще.
Этот код я не пропущу
phoneApp/src/keypad.js
Outdated
this.memory = this.memory + '-'; | ||
} | ||
|
||
this.number.children[1].textContent = this.memory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.number.children[1].textContent = this.memory;
выглядит как костыль. Тебе нужно конкретно получить элемент и изменять его.
children[1] - сегодня он равняется одному, завтра другому... и твой код может поломаться, это ненадежно
phoneApp/src/keypad.js
Outdated
// <span class="glyphicon glyphicon-circle-arrow-left" aria-hidden="true"></span>`; | ||
// number.innerHTML += strNumber; | ||
|
||
//var arr = ['1', '2', '3', '4', '5', '6', '7', '8', '9', "'*'", '0', "'#'"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
удали пожалуйста все комментарии и неиспользуемый код
https://github.com/AlexeySkydan201/phoneApp/tree/gh-pages
https://alexeyskydan201.github.io/phoneApp/
-- ? --
вт, 14 авг. 2018 г. в 10:58, Oleg Lustenko <[email protected]>:
… ***@***.**** requested changes on this pull request.
------------------------------
In phoneApp/src/edit_contacts.js
<#33 (comment)>
:
> this.container1 = document.getElementById('container');
this.scroll = document.getElementById('scroll');
this.arrEditMainInfo = ['First Name', 'Last Name', 'Company'];
- this.arrAdd = ['add home phone', 'add email', 'add address', 'add birthday', 'add social profile', 'add field']
+ this.arrAdd = ['add fullName', 'add email', 'add birthdate', 'add address', 'add gender ']
вместо массива лучше сделать статические декларативные данные в верстке.
Массив превращает в одном из мест твой код в циклы и ненужными условиями -
это не тот сценарий где нужно бояться дублирования кода.
Вот создание каждого инпута - это можно вынести "вроде отдельного
компонента"
------------------------------
In phoneApp/src/edit_contacts.js
<#33 (comment)>
:
> this.container1 = document.getElementById('container');
this.scroll = document.getElementById('scroll');
this.arrEditMainInfo = ['First Name', 'Last Name', 'Company'];
- this.arrAdd = ['add home phone', 'add email', 'add address', 'add birthday', 'add social profile', 'add field']
+ this.arrAdd = ['add fullName', 'add email', 'add birthdate', 'add address', 'add gender ']
+ }
+ render(id) {
+ console.log(`nnn`, id)
+ this.bd.forEach(element => {
+ if (element._id == id) {
+ this.arrAdd[0] = 'name - ' + element.fullName;
доступ по индексу это плохая практика, этого нужно избегать
------------------------------
In phoneApp/src/footer.js
<#33 (comment)>
:
> - </a>
- <a href="edit-contact.html" class="tab">
- <span class="glyphicon glyphicon-pencil" aria-hidden="true"></span>
- <span class="tab-text">Edit contact</span>
- </a>
- <a href="user.html" class="tab">
- <span class="glyphicon glyphicon-user" aria-hidden="true"></span>
- <span class="tab-text">User</span>
- </a>
- <a href="add-user.html" class="tab">
- <span class="glyphicon glyphicon-plus" aria-hidden="true"></span>
- <span class="tab-text">Add user</span>
- </a>
-</nav>
+ <nav class="main-nav" id = "nav1">
+ <a href="" class="tab active">
<a href="keypad.html" class="tab">
<span class="glyphicon glyphicon-th" aria-hidden="true"></span>
<span class="tab-text">Keypad</span>
</a>```
а вот это хороший кандидат чтобы нарисовать ссылку
------------------------------
In phoneApp/src/footer.js
<#33 (comment)>
:
> + <span class="glyphicon glyphicon-th" aria-hidden="true"></span>
+ <span class="tab-text">Keypad</span>
+ </a>
+ <a href="edit-contact.html" class="tab">
+ <span class="glyphicon glyphicon-pencil" aria-hidden="true"></span>
+ <span class="tab-text">Edit contact</span>
+ </a>
+ <a href="user.html" class="tab">
+ <span class="glyphicon glyphicon-user" aria-hidden="true"></span>
+ <span class="tab-text">User</span>
+ </a>
+ <a href="add-user.html" class="tab">
+ <span class="glyphicon glyphicon-plus" aria-hidden="true"></span>
+ <span class="tab-text">Add user</span>
+ </a>
+ </nav>
</div>`
const footer = document.getElementById('fo-er')
этот код нужно сделать классом, назвать роутинг(т.е переходы по страницам)
и у него вызываьт методы
------------------------------
In phoneApp/src/keypad.js
<#33 (comment)>
:
> - memory += `${num}`;
- console.log(memory.length)
- if (memory === '#' || memory === '*') {
- memory = memory;
- } else if (memory.length == 1) {
- memory = '(' + memory;
- } else if (memory.length == 4) {
- memory = memory + ')';
- } else if (memory.length == 8) {
- memory = memory + '-';
- } else if (memory.length == 11) {
- memory = memory + '-';
+class Keypad {
+ constructor() {
+ this.lol = document.getElementById('lol');
+ this.arr = ['1', '2', '3', '4', '5', '6', '7', '8', '9', "'*'", '0', "'#'"];
цифры у нас всегда статичны и создавать из них массив нет смысла.
Данные которые приходят с сервера или меняются состояние приложения -
необходимо хранить в структурах данных, остальные данные в 95% случаев это
статика или декларативный код
// императивный вариант - менее читабельныйconst numbers = [1,2,3]const numbersData = numbers.reduce((allNumbers, number) => allNumber + `<li>number</li>`, ``)
декларативный вариантconst numbersData_v2 = ` <li>1</li> <li>2</li> <li>3</li>`
второй случай очевиднее, нам не нужны калькуляции для этого, потому что у
нас никогда не появится цифра "11" например
------------------------------
In phoneApp/src/keypad.js
<#33 (comment)>
:
> + <div class="keypad-holder" id="key">
+ ${this.strKey()}
+ ${this.buttonCall}
+ </div>
+ </main>`
+ }
+ strKey() {
+ return this.arr.reduce(function(sum, current) {
+ return sum + `<button class="key" onclick = "app.pages.keypad.keyOut(${current})">${current}</button>`;
+ }, '');
+ }
+ keyOut(num) {
+ this.memory += `${num}`;
+ if (this.memory === '#' || this.memory === '*') {
+ this.memory = this.memory;
+ } else if (this.memory.length == 1) {
это нужно сделать при помощи regExp, можно гуглить, общаться в чате или
как-то еще.
Этот код я не пропущу
------------------------------
In phoneApp/src/keypad.js
<#33 (comment)>
:
> + }
+ keyOut(num) {
+ this.memory += `${num}`;
+ if (this.memory === '#' || this.memory === '*') {
+ this.memory = this.memory;
+ } else if (this.memory.length == 1) {
+ this.memory = '(' + this.memory;
+ } else if (this.memory.length == 4) {
+ this.memory = this.memory + ')';
+ } else if (this.memory.length == 8) {
+ this.memory = this.memory + '-';
+ } else if (this.memory.length == 11) {
+ this.memory = this.memory + '-';
+ }
+
+ this.number.children[1].textContent = this.memory;
this.number.children[1].textContent = this.memory;
выглядит как костыль. Тебе нужно конкретно получить элемент и изменять его.
children[1] - сегодня он равняется одному, завтра другому... и твой код
может поломаться, это ненадежно
------------------------------
In phoneApp/src/keypad.js
<#33 (comment)>
:
> }
-var key = document.getElementById('key');
-var strKey = arr.reduce(function(sum, current) {
+var key = new Keypad();
+//key.render();
+
+// var number = document.getElementById('number');
+// var strNumber = `<span class="glyphicon glyphicon-plus-sign" aria-hidden="true"></span>
+// <span class="numbers" >(050)5005050</span>
+// <span class="glyphicon glyphicon-circle-arrow-left" aria-hidden="true"></span>`;
+// number.innerHTML += strNumber;
+
+//var arr = ['1', '2', '3', '4', '5', '6', '7', '8', '9', "'*'", '0', "'#'"];
удали пожалуйста все комментарии и неиспользуемый код
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#33 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Al7sYasI1dzzZQbQeefpaR6rhPOW13zfks5uQoM1gaJpZM4V7Myv>
.
|
No description provided.