-
-
Notifications
You must be signed in to change notification settings - Fork 585
Fix how Bootstrap-Notfy is used by Pi-hole #2786
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
Conversation
Signed-off-by: DL6ER <dl6er@dl6er.de>
function showAlert(type, icon, title, message) { | ||
var opts = {}; | ||
title = " <strong>" + title + "</strong><br>"; | ||
const options = { |
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.
We use const
here. I will details the why below in case anyone doesn't know (I will admit: I didn't know).
In JavaScript, const
is used to declare a variable that cannot be reassigned. However, it does not make the variable immutable. This means that while you cannot reassign a const
variable to a new value, you can still modify the properties of an object that is assigned to a const
variable.
Here, the options
and settings
objects are declared using const
. This means that you cannot reassign them to a new value. However, the properties of these objects can still be modified.
For example, the options
and settings
objects are modified based on the type
parameter. The icon
property of the options
object is set to a default value if it is not provided, and the delay
property of the settings
object is multiplied by 2 if the type
is "warning" or "error".
In summary, const
in JavaScript does not make objects immutable, it only prevents the variable from being reassigned to a new value which seems a bit counter-intuitive given the name "const".
@@ -194,7 +194,6 @@ mg.include('scripts/pi-hole/lua/header_authenticated.lp','r') | |||
<script src="<?=pihole.fileversion('scripts/vendor/bootstrap-select.min.js')?>"></script> | |||
<script src="<?=pihole.fileversion('scripts/pi-hole/js/ip-address-sorting.js')?>"></script> | |||
<script src="<?=pihole.fileversion('scripts/vendor/daterangepicker.min.js')?>"></script> | |||
<script src="<?=pihole.fileversion('scripts/pi-hole/js/utils.js')?>"></script> |
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.
utils.js
is already included on all pages from the header. No need to do it a second time here.
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.
Tested OK
I'm not sure if this PR broke it, but I thought we had some kind of fade in/out animation for the notifications. But they don't fade anymore... Edit |
What does this implement/fix?
See title, this follows the documentation at https://grotesquegentleadvance--samkhaled.repl.co/
Related issue or feature (if applicable): N/A
Pull request in docs with documentation (if applicable): N/A
By submitting this pull request, I confirm the following:
git rebase
)Checklist:
developmental
branch.