From: Simon Tatham <anakin@pobox.com>
Date: Fri, 5 Apr 2013 15:49:29 +0000 (+0000)
Subject: Regretfully remove my trickery with a hidden <option> element inside
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=36f35f5db81d95c3fc038f27ac791a2f4f748c10;p=sgt-puzzles.git

Regretfully remove my trickery with a hidden <option> element inside
the game-type <select>, since IE turns out to ignore display:none on
options. Oh well.

Instead I now do a more transparent thing: when custom game params are
in use, there's a "Custom" option selected in the dropdown, and a
separate 'Re-customise' option which brings the config box back up.
When an ordinary preset is selected, the Custom option is missing, and
there's just a 'Customise'.

In the process I've tinkered a bit to arrange that the custom 'preset'
is always represented by a negative number rather than one past the
last real preset; that seems more consistent overall.

[originally from svn r9811]
---

diff --git a/emcc.c b/emcc.c
index d85a0e8..baf12d3 100644
--- a/emcc.c
+++ b/emcc.c
@@ -53,6 +53,7 @@
  *    that using whatever they normally use to print PDFs!)
  */
 
+#include <assert.h>
 #include <string.h>
 
 #include "puzzles.h"
@@ -528,14 +529,14 @@ const struct drawing_api js_drawing = {
  * Presets and game-configuration dialog support.
  */
 static game_params **presets;
-static int custom_preset;
+static int npresets;
 int have_presets_dropdown;
 
 void select_appropriate_preset(void)
 {
     if (have_presets_dropdown) {
         int preset = midend_which_preset(me);
-        js_select_preset(preset < 0 ? custom_preset : preset);
+        js_select_preset(preset < 0 ? -1 : preset);
     }
 }
 
@@ -656,7 +657,7 @@ void command(int n)
       case 2:                          /* game parameter dropdown changed */
         {
             int i = js_get_selected_preset();
-            if (i == custom_preset) {
+            if (i < 0) {
                 /*
                  * The user selected 'Custom', so launch the config
                  * box.
@@ -668,12 +669,14 @@ void command(int n)
                  * The user selected a preset, so just switch straight
                  * to that.
                  */
+                assert(i < npresets);
                 midend_set_params(me, presets[i]);
                 midend_new_game(me);
                 resize();
                 midend_redraw(me);
                 update_undo_redo();
                 js_focus_canvas();
+                select_appropriate_preset(); /* sort out Custom/Customise */
             }
         }
         break;
@@ -762,12 +765,10 @@ int main(int argc, char **argv)
 
     /*
      * Set up the game-type dropdown with presets and/or the Custom
-     * option. We remember the index of the Custom option (as
-     * custom_preset) so that we can easily treat it specially when
-     * it's selected.
+     * option.
      */
-    custom_preset = midend_num_presets(me);
-    if (custom_preset == 0) {
+    npresets = midend_num_presets(me);
+    if (npresets == 0) {
         /*
          * This puzzle doesn't have selectable game types at all.
          * Completely remove the drop-down list from the page.
@@ -777,8 +778,8 @@ int main(int argc, char **argv)
     } else {
         int preset;
 
-        presets = snewn(custom_preset, game_params *);
-        for (i = 0; i < custom_preset; i++) {
+        presets = snewn(npresets, game_params *);
+        for (i = 0; i < npresets; i++) {
             char *name;
             midend_fetch_preset(me, i, &name, &presets[i]);
             js_add_preset(name);
diff --git a/emcclib.js b/emcclib.js
index b648684..e0c4bea 100644
--- a/emcclib.js
+++ b/emcclib.js
@@ -71,7 +71,7 @@ mergeInto(LibraryManager.library, {
      * case we need to do something special - see below.
      */
     js_add_preset: function(ptr) {
-        var name = (ptr == 0 ? "Custom..." : Pointer_stringify(ptr));
+        var name = (ptr == 0 ? "Customise..." : Pointer_stringify(ptr));
         var value = gametypeoptions.length;
 
         var option = document.createElement("option");
@@ -81,39 +81,20 @@ mergeInto(LibraryManager.library, {
         gametypeoptions.push(option);
 
         if (ptr == 0) {
-            // Create a _second_ element called 'Custom', which is
-            // hidden.
-            //
-            // Hiding this element (that is, setting it display:none)
-            // has the effect of making it not show up when the
-            // drop-down list is actually opened, but still show up
-            // when the item is selected.
-            //
-            // So what happens is that there's one element marked
-            // 'Custom' that the _user_ selects, but a second one to
-            // which we reset the dropdown after the config box
-            // returns (if we don't then turn out to select a
-            // different preset anyway). The point is that if the user
-            // has 'Custom' selected, but then wants to customise
-            // their settings a second time, we still get an onchange
-            // event when they select the Custom option again, which
-            // we wouldn't get if the browser thought it was already
-            // the selected one. But here, it's _not_ the selected
-            // option already; its invisible evil twin is selected.
-            //
-            // (Actually, they're not _identical_ evil twins: we label
-            // the two slightly differently. The visible one that the
-            // user can select is labelled "Custom..." to hint that it
-            // opens a dialog box, whereas the invisible one that's
-            // left shown after the box closes is just "Custom",
-            // because that's telling you what you _have_ got
-            // selected.)
+            // The option we've just created is the one for inventing
+            // a new custom setup.
+            gametypenewcustom = option;
+            option.value = -1;
+
+            // Now create another element called 'Custom', which will
+            // be auto-selected by us to indicate the custom settings
+            // you've previously selected. However, we don't add it to
+            // the game type selector; it will only appear when the
+            // user actually has custom settings selected.
             option = document.createElement("option");
-            option.value = value;
+            option.value = -2;
             option.appendChild(document.createTextNode("Custom"));
-            option.style.display = "none";
-            gametypeselector.appendChild(option);
-            gametypehiddencustom = option;
+            gametypethiscustom = option;
         }
     },
 
@@ -140,11 +121,31 @@ mergeInto(LibraryManager.library, {
      * which turn out to exactly match a preset).
      */
     js_select_preset: function(n) {
-        if (gametypeoptions[n].value == gametypehiddencustom.value) {
-            // If we're asked to select the visible Custom option,
-            // select the invisible one instead. See comment above in
-            // js_add_preset.
-            gametypehiddencustom.selected = true;
+        if (gametypethiscustom !== null) {
+            // Fiddle with the Custom/Customise options. If we're
+            // about to select the Custom option, then it should be in
+            // the menu, and the other one should read "Re-customise";
+            // if we're about to select another one, then the static
+            // Custom option should disappear and the other one should
+            // read "Customise".
+
+            if (gametypethiscustom.parentNode == gametypeselector)
+                gametypeselector.removeChild(gametypethiscustom);
+            if (gametypenewcustom.parentNode == gametypeselector)
+                gametypeselector.removeChild(gametypenewcustom);
+
+            if (n < 0) {
+                gametypeselector.appendChild(gametypethiscustom);
+                gametypenewcustom.lastChild.data = "Re-customise...";
+            } else {
+                gametypenewcustom.lastChild.data = "Customise...";
+            }
+            gametypeselector.appendChild(gametypenewcustom);
+            gametypenewcustom.selected = false;
+        }
+
+        if (n < 0) {
+            gametypethiscustom.selected = true;
         } else {
             gametypeoptions[n].selected = true;
         }
diff --git a/emccpre.js b/emccpre.js
index 701c7f7..4ce2209 100644
--- a/emccpre.js
+++ b/emccpre.js
@@ -83,13 +83,18 @@ var dlg_return_sval, dlg_return_ival;
 // list of the <option> objects inside it. Used by js_add_preset(),
 // js_get_selected_preset() and js_select_preset().
 //
-// gametypehiddencustom is a second copy of the 'Custom' dropdown
-// element, set to display:none. This is used by a bodge in emcclib.js
-// (see comment in js_add_preset) to arrange that if the Custom
-// element is (apparently) already selected, we still find out if the
-// user selects it again.
+// gametypethiscustom is an option which indicates some custom game
+// params you've already set up, and which will be auto-selected on
+// return from the customisation dialog; gametypenewcustom is an
+// option which you select to indicate that you want to bring up the
+// customisation dialog and select a new configuration. Ideally I'd do
+// this with just one option serving both purposes, but instead we
+// have to do this a bit oddly because browsers don't send 'onchange'
+// events for a select element if you reselect the same one - so if
+// you've picked a custom setup and now want to change it, you need a
+// way to specify that.
 var gametypeselector = null, gametypeoptions = [];
-var gametypehiddencustom = null;
+var gametypethiscustom = null, gametypehiddencustom = null;
 
 // The two anchors used to give permalinks to the current puzzle. Used
 // by js_update_permalinks().