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 -->


AltStyle によって変換されたページ (->オリジナル) /