Conversation
UlisesGascon
left a comment
UlisesGascon
left a comment
There was a problem hiding this comment.
En general muy bien, pero sería interesante darle un repaso con lo que hemos visto estos últimos meses. Creo que podrías darle un enfoque más limpio y funcional. Aunque tecnicamete esta bien, al margen de los warnings... creo que falta un poco más de arquitectura. Pero muy bien, me gusta como enfocas el reto 😉
| @@ -0,0 +1,198 @@ | |||
| (function() { | |||
| const inputs = document.querySelectorAll('input[type=text]'); | |||
| const containerList = document.querySelector('#containerList'); | |||
There was a problem hiding this comment.
document.querySelector es infinitamente más lento que document.getElementById.
📖 Benchmarkhttps://jsperf.com/getelementbyid-vs-queryselector/25
| const buttonDelete = document.querySelector('#delete'); | ||
| const buttonDeleteAll = document.querySelector('#deleteAll'); | ||
|
|
||
| buttonSave.addEventListener('click', saveContact, false); |
There was a problem hiding this comment.
Muy buena abstracción! 👍
| let contactsToStorage = []; | ||
| let indexToChange = null; | ||
|
|
||
| function onShowDeleteAllButton(show = false) { |
There was a problem hiding this comment.
onShowDeleteAllButton y onShowDeleteButton son super similares. Usa un closure o pasarle un argumento para evitar repetir tanto código
| buttonDelete.addEventListener('click', deleteContact, false); | ||
| buttonDeleteAll.addEventListener('click', deleteAllContact, false); | ||
|
|
||
| let contactsToStorage = []; |
There was a problem hiding this comment.
| data.forEach(item => appendTemplate(item)); | ||
| } | ||
|
|
||
| function appendTemplate(contactValues) { |
There was a problem hiding this comment.
contactData o data o simplemente contact pienso que sería semánticamente más esclarecedor 😉
|
|
||
| function saveContact() { | ||
| const values = _getValuesForm(); | ||
| const isFormOk = _isFormOK(values); |
There was a problem hiding this comment.
isFormOk la usa solo una vez.. tiene sentido crear una variable?
| const PHONE_REGEX = /[0-9]+/g; | ||
| const EMAIL_REGEX = /([\w]+[\-]*[0-9]*)+@(([\w]{3,})+[\.])+([a-z]{2,3})/g; | ||
|
|
||
| if (values[0].match(NAME_REGEX) === null || values[1].match(PHONE_REGEX) === null || values[2].match(EMAIL_REGEX) === null) { |
There was a problem hiding this comment.
esto debe limpiarse un poco.. con un array o similar. Enfoque más funcional
2 participants
agenda de contactos con localstorage