We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
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
Hey Max, while checking-out your project, I noticed some things that could be improved. If you want, I can submit a pull request for these things.
notificationJS/notificationJS.js
Lines 38 to 39 in ee11974
const
Line 40 in ee11974
title.innerHTML = this.info.title;
Lines 45 to 46 in ee11974
if
Lines 55 to 56 in ee11974
const description = ...
Line 59 in ee11974
Line 80 in ee11974
const notification = ...
Lines 82 to 84 in ee11974
setTimeout(() => notification.remove(), this.info.animation_duration * 1000 + 100)
Lines 99 to 100 in ee11974
document.body
Line 103 in ee11974
this.info.animation_duration
Lines 111 to 118 in ee11974
hide
Line 123 in ee11974
Lines 131 to 133 in ee11974
The text was updated successfully, but these errors were encountered:
No branches or pull requests
Hey Max,
while checking-out your project, I noticed some things that could be improved.
If you want, I can submit a pull request for these things.
notificationJS/notificationJS.js
Lines 38 to 39 in ee11974
Can be
const
notificationJS/notificationJS.js
Line 40 in ee11974
title.innerHTML = this.info.title;
?notificationJS/notificationJS.js
Lines 45 to 46 in ee11974
Can be joined into a single
if
.notificationJS/notificationJS.js
Lines 55 to 56 in ee11974
Can be simplified to
const description = ...
notificationJS/notificationJS.js
Line 59 in ee11974
Should be moved to into the block where the link is created.
notificationJS/notificationJS.js
Line 80 in ee11974
Can be simplified to
const notification = ...
. Or just this.notification can be used.notificationJS/notificationJS.js
Lines 82 to 84 in ee11974
setTimeout(() => notification.remove(), this.info.animation_duration * 1000 + 100)
notificationJS/notificationJS.js
Lines 99 to 100 in ee11974
Can use
document.body
notificationJS/notificationJS.js
Line 103 in ee11974
The animation duration is configurable, however the opacity transition duration not adapted to
this.info.animation_duration
notificationJS/notificationJS.js
Lines 111 to 118 in ee11974
The hiding logic could be moved into a
hide
method, which can then also be used in the close icon event listener.notificationJS/notificationJS.js
Line 123 in ee11974
Makes the flag kind of unnecessary 😄
notificationJS/notificationJS.js
Lines 131 to 133 in ee11974
These
if
s can be joined.The text was updated successfully, but these errors were encountered: