Merge pull request #1111 from gusthoff/topic/issues/1100

Widget: fix dangerous menu options

Author: gusthoff

frontend/sphinx/widget/templates/widget.html

@@ -11,19 +11,6 @@
     {% for f in w.files %}
         <div class="file" data-basename="{{ f.basename }}">{{ f.content }}</div>
     {% endfor %}
-    <div id="{{ w.id }}.tab" class="tab">
-        {% for t in w.files %}
-            <button id="{{ w.id }}.tab.{{ t.basename }}" class="tab-links">{{ t.basename }}</button>
-        {% endfor %}
-    </div>
-    <div id="{{ w.id }}.editors">
-        <div id="{{ w.id }}.editors.editor" class="editor-container"></div>
-        <div id="{{ w.id }}.editors.non-tabbed-editor" class="non-tabbed-editor-container" hidden>
-            {% for t in w.files %}
-                <div id="{{ w.id }}.editors.non-tabbed-editor.{{ t.basename }}" class="non-tabbed-links"></div>
-            {% endfor %}
-        </div>
-    </div>
     <div id="{{ w.id }}.settings-bar" class="settings-bar">
         <div class="dropdown-container settingsbar-item">
             <button class="dropdown-btn">
@@ -78,6 +65,19 @@
             Download
         </button>
     </div>
+    <div id="{{ w.id }}.editors">
+        <div id="{{ w.id }}.editors.editor" class="editor-container"></div>
+        <div id="{{ w.id }}.editors.non-tabbed-editor" class="non-tabbed-editor-container" hidden>
+            {% for t in w.files %}
+                <div id="{{ w.id }}.editors.non-tabbed-editor.{{ t.basename }}" class="non-tabbed-links"></div>
+            {% endfor %}
+        </div>
+    </div>
+    <div id="{{ w.id }}.tab" class="tab">
+        {% for t in w.files %}
+            <button id="{{ w.id }}.tab.{{ t.basename }}" class="tab-links">{{ t.basename }}</button>
+        {% endfor %}
+    </div>
     <div class="row output-row">
         <div id="{{ w.id }}.button-group" class="col-md-3 button-group">
             {% for b in w.button_group %}

frontend/src/styles/learn.scss

@@ -160,6 +160,8 @@ pre.widget {
     flex-wrap: nowrap;
     overflow-x: auto;
     -webkit-overflow-scrolling: touch;
+    margin-bottom: 10px;
+
     button {
         background-color: inherit;
         float: left;
@@ -196,7 +198,6 @@ pre.widget {
     padding-left: 13px;
     display: block;
     margin-top: 0px;
-    margin-bottom: 10px;
 
     button {
         background-color: transparent;

frontend/src/ts/strings.ts

@@ -5,6 +5,11 @@ export const RESET_CONFIRM_MSG =
 'Your changes will be lost after reset. ' +
 'Are you sure you want to reset the editor and the compiler options?';
 
+export const RELOAD_CONFIRM_MSG =
+'Changing this setting requires reloading the page: ' +
+'your changes will be lost. ' +
+'Are you sure you want to proceed?';
+
 export const DOWNLOAD_TOOLTIP = 'Download source files';
 export const DOWNLOAD_MAINTENANCE =
 'The download functionilty is currently undergoing maintenance';

frontend/src/ts/widget.ts

@@ -151,16 +151,23 @@ class Widget {
     this.setTabbedView(tabbedView);
 
     tabSetting.addEventListener('change', () => {
-      let tabbedView = 'true';
-      if (!tabSetting.checked) {
-        tabbedView = 'false';
-      }
-      cookies.set('tabbed_view', tabbedView, {expires: 3650});
-      this.setTabbedView(tabSetting.checked);
+      if (window.confirm(Strings.RELOAD_CONFIRM_MSG)) {
+        let tabbedView = 'true';
+        if (!tabSetting.checked) {
+          tabbedView = 'false';
+        }
+        cookies.set('tabbed_view', tabbedView, {expires: 3650});
+        this.setTabbedView(tabSetting.checked);
 
-      // Current approach: just reload the page to
-      // set the correct theme for all widgets.
-      location.reload();
+        // Current approach: just reload the page to
+        // set the correct theme for all widgets.
+        location.reload();
+      }
+      else
+      {
+        // Revert if user chooses "no change"
+        tabSetting.checked = !tabSetting.checked;
+      }
     });
 
     this.initCompilerSwitches();
@@ -175,16 +182,23 @@ class Widget {
     this.setTheme(cookieTheme);
 
     themeSetting.addEventListener('change', () => {
-      let theme = 'light';
-      if (themeSetting.checked) {
-        theme = 'dark';
-      }
-      cookies.set('theme', theme, {expires: 3650});
-      this.setTheme(theme);
+      if (window.confirm(Strings.RELOAD_CONFIRM_MSG)) {
+        let theme = 'light';
+        if (themeSetting.checked) {
+          theme = 'dark';
+        }
+        cookies.set('theme', theme, {expires: 3650});
+        this.setTheme(theme);
 
-      // Current approach: just reload the page to
-      // set the correct theme for all widgets.
-      location.reload();
+        // Current approach: just reload the page to
+        // set the correct theme for all widgets.
+        location.reload();
+      }
+      else
+      {
+        // Revert if user chooses "no change"
+        themeSetting.checked = !themeSetting.checked;
+      }
     });
 
     const resetButton =

frontend/tests/ts/widget.spec.ts

@@ -404,37 +404,6 @@ describe('Widget', () => {
         editor = ace.edit(editorDiv);
       });
 
-      it('should have a checkbox that switches editor theme', () => {
-        const box = getElemById(root.id + '.settings-bar.theme-setting') as
-          HTMLInputElement;
-        const origTheme = editor.getTheme();
-        box.checked = !box.checked;
-        triggerEvent(box, 'change');
-        expect(editor.getTheme()).to.not.equal(origTheme);
-      });
-
-      it('should have a checkbox that switches tab setting', () => {
-        const nonTabbedEditorDiv: HTMLElement = getElemById(root.id + '.editors.non-tabbed-editor');
-        const box = getElemById(root.id + '.settings-bar.tab-setting') as
-            HTMLInputElement;
-
-        const origIsTabbedView : boolean = !editorDiv.hidden;
-        expect(nonTabbedEditorDiv.hidden).to.equal(origIsTabbedView);
-
-        box.checked = !box.checked;
-        triggerEvent(box, 'change');
-        expect(editorDiv.hidden).to.equal(origIsTabbedView);
-        expect(nonTabbedEditorDiv.hidden).to.not.equal(origIsTabbedView);
-
-        //  Test hiding / showing buttons for tabbed view
-        const tabs = getElemById(root.id + '.tab');
-        const headers = getElemsByTag(tabs, 'button');
-
-        for (const h of headers) {
-          expect(h.hidden).to.equal(origIsTabbedView);
-        }
-      });
-
       it('should have a button that resets the editor', () => {
         const btn = getElemById(root.id + '.settings-bar.reset-btn') as
           HTMLButtonElement;