Revision c6044dc3-3671-4e1d-901b-bfff7a0f1733 - Code Review Stack Exchange
##Consolidating the two functions
It appears that the only difference between `dialog_OkCancel` and `dialog_Input_OkCancel` is that it accepts the `input` parameter and passes that along. One could just combine those two into a single function and if the `input` parameter is defined, it will get passed along.
See the snippet below where the two functions are combined into a single function. Presuming that works, one could conceivably merge the functionality of that function into `dialog_Handler()`, but perhaps that wouldn't work because there are other functions that call that function...
###Edit:
In a comment, you asked:
>Do you know if there is any way to call `$(this).dialog("close");` without writing it inside each `function(){ }` of the `var buttons`?
Yes, you can abstract the function of closing the dialog out to a separate function, like:
function closeDialog() {
$(this).dialog("close");
}
And that function can be bound to the functions for the button handlers using the Javascript function [.bind()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind) to create a [partially applied function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind#Partially_applied_functions) for the `funk_Ok` parameter:
var buttons = {
"Ok": closeDialog.bind(null, funk_Ok),
"Cancel": closeDialog
}
And that close function could accept an optional callback function (for functions like `func_Ok`)- if that is a function, it can be called:
function closeDialog(callback) {
(typeof callback === "function") && callback();
$(this).dialog("close");
}
But the problem here is that `this` in this context is not the same as before - it is the `window` object. So one solution is to give the HTML element wrapped in the dialog object an [_id_ attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id), like below:
const dialogId = 'message_dialog';
function dialog_Handler({
title,
message,
buttons,
input
}) {
$("<p id='"+dialogId+"'>" + message + "</p>").dialog({ // Could use “var dialog = ”
And then in the `closeDialog` function, just reference that [_id_ attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id) when closing the dialog:
function closeDialog(callback) {
(typeof callback === "function") && callback();
$('#'+dialogId).dialog("close");
}
See this in action in the snippet below:
<!-- begin snippet: js hide: true console: true babel: false -->
<!-- language: lang-js -->
const dialogId = 'message_dialog';
// jQuery UI dialog custom management
function dialog_Handler({
title,
message,
buttons,
input
}) {
$("<p id='"+dialogId+"'>" + message + "</p>").dialog({ // Could use “var dialog = ”
//autoOpen: false, // false would prevents regular opening
show: "drop",
open: function(event, ui) {
// Overlay parameterization
$("div.ui-widget-overlay").css({
"background": "#000",
"opacity": "0.4"
});
$(".ui-widget-overlay").hide().fadeIn();
// Adds some more configuration if input needed
if (input) {
// Adds input field right under the message
$(this).append('<br /><br /><input id="dialog_Input" style="width: 350px; padding: 4px;" type="text" placeholder="' + input + '"><br />');
// Binds “Enter” to press first button (usually “Ok”)
$(this).keydown(function(event) {
if (event.keyCode == $.ui.keyCode.ENTER) {
$(this).parent().find("button:eq(1)").trigger("click");
return false;
}
});
}
},
title: title,
buttons: buttons,
modal: true,
resizable: false,
height: "auto",
width: 400,
hide: {
effect: "drop",
duration: "fast"
},
closeOnEscape: true,
// Overlay fadeout
beforeClose: function(event, ui) {
const widgetOverlay = $('.ui-widget-overlay');
// Wait for the overlay to be faded out to try closing again
if (widgetOverlay.is(":visible")) {
widgetOverlay.fadeOut("fast", function() {
widgetOverlay.hide();
$('.ui-icon-closethick').trigger('click');
});
return false;
}
},
close: function() {
$(this).dialog("destroy");
}
});
return;
}
function closeDialog(callback) {
(typeof callback === "function") && callback();
$('#'+dialogId).dialog("close");
}
//function dialog_Input_OkCancel({
function dialog_OkCancel({
title,
message,
input,
funk_Ok
}) {
var buttons = {
"Ok": closeDialog.bind(null, funk_Ok)/*function() {
funk_Ok();
$(this).dialog("close");
}*/,
"Cancel": closeDialog/*function() {
$(this).dialog("close");
}*/
}
dialog_Handler({
title,
message,
buttons,
input
});
return;
}
<!-- language: lang-html -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.12.1/jquery-ui.min.js"></script>
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.12.1/jquery-ui.min.css" />
<button onclick='dialog_OkCancel({
title: "Title",
message: "I do not have any idea for the message.",
funk_Ok: function(){ console.log("Sorry. :("); }
});'>Dialog Ok/Cancel</button>
<button onclick='dialog_OkCancel({
title: "Title",
message: "I do not have any idea for the message.",
input: "Type something…",
funk_Ok: function(){ console.log("Give me more words ! :)"); }
});'>Dialog Input</button>
<!-- end snippet -->
##Other feedback
###Cache DOM references
I see a couple places where DOM elements are looked up in succession, for example:
> if ($('.ui-widget-overlay').is(":visible")) {
$('.ui-widget-overlay').fadeOut("fast", function() {
$('.ui-widget-overlay').hide();
$('.ui-icon-closethick').trigger('click');
});
return false;
}
It is wise to cache those lookups in a variable - or actually, use [`const`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const) to declare a constant unless there is a need to re-assign that value...
const widgetOverlay = $('.ui-widget-overlay');
// Wait for the overlay to be faded out to try closing again
if (widgetOverlay.is(":visible")) {
widgetOverlay.fadeOut("fast", function() {
widgetOverlay.hide();
$('.ui-icon-closethick').trigger('click');
});
return false;
}
###Placeholder text
I would use the [`placeholder`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-placeholder) attribute instead of the `value` attribute of the text input, since it appears that text is really prompting the user...
<!-- begin snippet: js hide: false console: true babel: false -->
<!-- language: lang-html -->
<input type="text" placeholder="Type some text here..." />
<!-- end snippet -->