-
Notifications
You must be signed in to change notification settings - Fork 40
Яковлев Илья M3339 #18
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
base: master
Are you sure you want to change the base?
Conversation
|
Новое сообщение добавляется нажатием 'n', старые сообщения удаляются на 'd'. |
js/app.js
Outdated
| } | ||
|
|
||
| var currentMessage = messages[index]; | ||
| currentMessage.setAttribute("state", "deleted"); |
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.
Кастомные аттрибуты у узлов лучше сторить в неймспейсе data-*
js/app.js
Outdated
|
|
||
| var currentMessage = messages[index]; | ||
| currentMessage.setAttribute("state", "deleted"); | ||
| var parent = currentMessage.parentNode; |
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.
Для чего данная переменная?
css/animations.css
Outdated
| border-width: 0px; | ||
| } | ||
|
|
||
| @keyframes appearance { |
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.
Можно анимацию для добавления письма организовать через transition
js/app.js
Outdated
|
|
||
| function newMessage() { | ||
| var currentMessage = document.getElementsByClassName("mailbox__mail")[0]; | ||
| if (currentMessage.getAttribute("state") != "hidden") { |
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.
Хорошо бы проработать кейс добавления не одного письма а нескольких
| @@ -0,0 +1,36 @@ | |||
| document.onkeypress = function(event) { | |||
| if (event.key == 'n') { | |||
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.
Кроме клавиатурных сокращений попробуй организовать удаление писем по выделенным при нажатии на кнопку-ссылку Удалить
js/app.js
Outdated
| return; | ||
| } | ||
| currentMessage.setAttribute("state", "showing"); | ||
| setTimeout(function() { |
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.
Можно повесить обработчик на событие окончания анимации animationend
No description provided.