Merge remote-tracking branch 'smblott-github/option-for-options-page'

Conflicts:
	pages/options.html

3 files changed, 135 insertions, 92 deletions

diff --git a/pages/options.coffee b/pages/options.coffee
index 3474bcba..cd19fa37 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -42,13 +42,7 @@ class Option
   @saveOptions: ->
     Option.all.map (option) -> option.save()
     $("saveOptions").disabled = true
-
-  # Used by text options. <ctrl-Enter> saves all options.
-  activateCtrlEnterListener: (element) ->
-    element.addEventListener "keyup", (event) ->
-      if event.ctrlKey and event.keyCode == 13
-        element.blur()
-        Option.saveOptions()
+    $("saveOptions").innerHTML = "No Changes"
 
   # Abstract method; only implemented in sub-classes.
   # Populate the option's DOM element (@element) with the setting's current value.
@@ -66,7 +60,6 @@ class TextOption extends Option
   constructor: (field,enableSaveButton) ->
     super(field,enableSaveButton)
     @element.addEventListener "input", enableSaveButton
-    @activateCtrlEnterListener @element
   populateElement: (value) -> @element.value = value
   readValueFromElement: -> @element.value.trim()
 
@@ -74,7 +67,6 @@ class NonEmptyTextOption extends Option
   constructor: (field,enableSaveButton) ->
     super(field,enableSaveButton)
     @element.addEventListener "input", enableSaveButton
-    @activateCtrlEnterListener @element
 
   populateElement: (value) -> @element.value = value
   # If the new value is not empty, then return it. Otherwise, restore the default value.
@@ -89,7 +81,6 @@ class ExclusionRulesOption extends Option
     super(args...)
     $("exclusionAddButton").addEventListener "click", (event) =>
       @appendRule { pattern: "", passKeys: "" }
-      @maintainExclusionMargin()
       # Focus the pattern element in the new rule.
       @element.children[@element.children.length-1].children[0].children[0].focus()
       # Scroll the new rule into view.
@@ -97,11 +88,8 @@ class ExclusionRulesOption extends Option
       exclusionScrollBox.scrollTop = exclusionScrollBox.scrollHeight
 
   populateElement: (rules) ->
-    while @element.firstChild
-      @element.removeChild @element.firstChild
     for rule in rules
       @appendRule rule
-    @maintainExclusionMargin()
 
   # Append a row for a new rule.
   appendRule: (rule) ->
@@ -111,7 +99,6 @@ class ExclusionRulesOption extends Option
     for field in ["pattern", "passKeys"]
       element = row.querySelector ".#{field}"
       element.value = rule[field]
-      @activateCtrlEnterListener element
       for event in [ "input", "change" ]
         element.addEventListener event, enableSaveButton
 
@@ -120,13 +107,12 @@ class ExclusionRulesOption extends Option
       row = event.target.parentNode.parentNode
       row.parentNode.removeChild row
       enableSaveButton()
-      @maintainExclusionMargin()
 
     @element.appendChild row
 
   readValueFromElement: ->
     rules =
-      for element in @element.children
+      for element in @element.getElementsByClassName "exclusionRuleTemplateInstance"
         pattern = element.children[0].firstChild.value.trim()
         passKeys = element.children[1].firstChild.value.trim()
         { pattern: pattern, passKeys: passKeys }
@@ -138,20 +124,11 @@ class ExclusionRulesOption extends Option
     flatten = (rule) -> if rule and rule.pattern then rule.pattern + "\n" + rule.passKeys else ""
     a.map(flatten).join("\n") == b.map(flatten).join("\n")
 
-  # Hack.  There has to be a better way than...
-  # The y-axis scrollbar for "exclusionRules" is only displayed if it is needed.  When visible, it appears on
-  # top of the enclosed content (partially obscuring it).  Here, we adjust the margin of the "Remove" button to
-  # compensate.
-  maintainExclusionMargin: ->
-    scrollBox = $("exclusionScrollBox")
-    margin = if scrollBox.clientHeight < scrollBox.scrollHeight then "16px" else "0px"
-    for element in scrollBox.getElementsByClassName "exclusionRemoveButton"
-      element.style["margin-right"] = margin
-
 #
 # Operations for page elements.
 enableSaveButton = ->
   $("saveOptions").removeAttribute "disabled"
+  $("saveOptions").innerHTML = "Save Changes"
 
 # Display either "linkHintNumbers" or "linkHintCharacters", depending upon "filterLinkHints".
 maintainLinkHintsView = ->
@@ -175,9 +152,13 @@ toggleAdvancedOptions =
         $("advancedOptionsLink").innerHTML = "Hide advanced options"
       advancedMode = !advancedMode
       event.preventDefault()
+      # Prevent the "advanced options" link from retaining the focus.
+      document.activeElement.blur()
 
 activateHelpDialog = ->
   showHelpDialog chrome.extension.getBackgroundPage().helpDialogHtml(true, true, "Command Listing"), frameId
+  # Prevent the "show help" link from retaining the focus.
+  document.activeElement.blur()
 
 #
 # Initialization.
@@ -214,3 +195,8 @@ document.addEventListener "DOMContentLoaded", ->
   maintainLinkHintsView()
   window.onbeforeunload = -> "You have unsaved changes to options." unless $("saveOptions").disabled
 
+  document.addEventListener "keyup", (event) ->
+    if event.ctrlKey and event.keyCode == 13
+      document.activeElement.blur() if document?.activeElement?.blur
+      Option.saveOptions()
+
diff --git a/pages/options.html b/pages/options.html
index 84953023..b52974d6 100644
--- a/pages/options.html
+++ b/pages/options.html
@@ -6,12 +6,14 @@
       body {
         font: 14px "DejaVu Sans", "Arial", sans-serif;
         color: #303942;
-        width: 680px;
         margin: 0 auto;
       }
       a, a:visited { color: #15c; }
       a:active { color: #052577; }
-      div#wrapper { width: 500px; }
+      div#wrapper, #footerWrapper {
+        width: 540px;
+        margin-left: 35px;
+      }
       header {
         font-size: 18px;
         font-weight: normal;
@@ -112,21 +114,11 @@
         width: 40px;
         margin-right: 3px;
       }
-      textarea#userDefinedLinkHintCss {
+      textarea#userDefinedLinkHintCss, textarea#keyMappings, textarea#searchEngines {
         width: 100%;;
         min-height: 130px;
         white-space: nowrap;
       }
-      textarea#keyMappings {
-        width: 100%;
-        min-height: 135px;
-        white-space: nowrap;
-      }
-      textarea#searchEngines {
-        width: 100%;
-        min-height: 135px;
-        white-space: nowrap;
-      }
       input#previousPatterns, input#nextPatterns {
         width: 100%;
       }
@@ -141,20 +133,13 @@
         font-size: 80%;
       }
       /* Make the caption in the settings table as small as possible, to pull the other fields to the right. */
-      td:first-child {
+      .caption {
         width: 1px;
         white-space: nowrap;
       }
       #buttonsPanel { width: 100%; }
       #advancedOptions { display: none; }
       #advancedOptionsLink { line-height: 24px; }
-      #saveOptions { float: right; }
-      #saveOptions { margin-right: 0; }
-      #showHelpDialogMessage {
-        width: 100%;
-        color: #979ca0;
-        font-size: 12px;
-      }
       .help {
         position: absolute;
         right: -320px;
@@ -180,34 +165,84 @@
       }
       /* Boolean options have a tighter form representation than text options. */
       td.booleanOption { font-size: 12px; }
-      footer {
-        padding: 15px 0;
-        border-top: 1px solid #eee;
-      }
       /* Ids and classes for rendering exclusionRules */
       #exclusionScrollBox {
         overflow: scroll;
         overflow-x: hidden;
         overflow-y: auto;
-        height: 170px;
-        border: 1px solid #bfbfbf;
+        max-height: 135px;
+        min-height: 75px;
         border-radius: 2px;
         color: #444;
+        width: 100%
       }
-      input.pattern, input.passKeys {
+      #exclusionScrollBox::-webkit-scrollbar {
+        width: 12px;
+      }
+      #exclusionScrollBox::-webkit-scrollbar-track {
+        -webkit-box-shadow: inset 0 0 6px rgba(0,0,0,0.3);
+        border-radius: 2px;
+      }
+
+      #exclusionScrollBox::-webkit-scrollbar-thumb {
+        border-radius: 2px;
+        -webkit-box-shadow: inset 0 0 6px rgba(0,0,0,0.5);
+      }
+      #exclusionRules {
+        width: 100%;
+      }
+      .exclusionRulePassKeys {
+        width: 33%;
+      }
+      .exclusionRemoveButton {
+        width: 1px; /* 1px; smaller than the button itself. */
+      }
+      .exclusionRemoveButtonButton {
+        border: none;
+        background-color: #fff;
+        color: #979ca0;
+      }
+      .exclusionRemoveButtonButton:hover {
+        color: #444;
+      }
+      input.pattern, input.passKeys, .exclusionHeaderText {
+        width: 100%;
         font-family: Consolas, "Liberation Mono", Courier, monospace;
         font-size: 14px;
       }
-      .pattern {
-        width: 250px;
-      }
-      .passKeys {
-        width: 120px;
+      .exclusionHeaderText {
+        padding-left: 3px;
+        color: #979ca0;
       }
       #exclusionAddButton {
         float: right;
-        margin-top: 5px;
         margin-right: 0px;
+        margin-top: 5px;
+      }
+      #footer {
+        background: #f5f5f5;
+        border-top: 1px solid #979ca0;
+        position: fixed;
+        bottom: 0px;
+        z-index: 10;
+      }
+      #footer, #footerTable, #footerTableData {
+        width: 100%;
+      }
+      #endSpace {
+        /* Leave space for the fixed footer. */
+        min-height: 30px;
+        max-height: 30px;
+      }
+      #helpText {
+        font-size: 12px;
+      }
+      #saveOptionsTableData {
+        float: right;
+      }
+      #saveOptions {
+        white-space: nowrap;
+        width: 110px;
       }
     </style>
 
@@ -224,24 +259,28 @@
           <td>
             <div class="help">
               <div class="example">
-                The left column contains URL patterns.  Vimium will be wholly or partially disabled for URLs
-                matching these patterns.  Patterns are Javascript regular expressions.  Additionally, the
-                character "*" matches any zero or more characters.
+                Wholly or partially disable Vimium.  "Patterns" are URL regular expressions;
+                additionally, "*" matches any zero or more characters.
                 <br/><br/>
-                The right column contains keys which Vimium would would normally handle, but which should
-                instead be passed through to the underlying web page (for pages matching the
-                pattern).  If left empty, then Vimium will be wholly disabled.
+                If "Keys" is left empty, then vimium is wholly disabled.
+                Otherwise, just the listed keys are disabled (they are passed through).
               </div>
             </div>
             <div>
                <div id="exclusionScrollBox">
-                  <table id="exclusionRules"></table>
+                  <table id="exclusionRules">
+                     <tr>
+                        <td><span class="exclusionHeaderText">Patterns</span></td>
+                        <td><span class="exclusionHeaderText">Keys</span></td>
+                     </tr>
+                  </table>
                   <template id="exclusionRuleTemplate">
-                    <tr>
-                       <td><input/ type="text" class="pattern" placeholder="URL pattern"></td>
-                       <td><input/ type="text" class="passKeys" placeholder="Exclude keys"></td>
-                       <td><input/ type="button" class="exclusionRemoveButton" tabindex = "-1" value="&#x2716;"></td>
-                    </tr>
+                     <tr class="exclusionRuleTemplateInstance">
+                        <td><input/ type="text" class="pattern" placeholder="URL pattern"></td>
+                        <td class="exclusionRulePassKeys"><input/ type="text" class="passKeys" placeholder="Exclude keys"></td>
+                        <td class="exclusionRemoveButton">
+                          <input/ type="button" tabindex = "-1" class="exclusionRemoveButtonButton" value="&#x2716;"></td>
+                     </tr>
                   </template>
                </div>
                <button id="exclusionAddButton">Add Rule</button>
@@ -283,22 +322,17 @@ unmapAll
           </td>
         </tr>
         <tr>
-          <td class="caption"></td>
-          <td verticalAlign="top" class="booleanOption">
-            <label>
-              <input id="smoothScroll" type="checkbox"/>
-              Use smooth scrolling.
-            </label>
-          </td>
-        </tr>
-        <tr>
-           <td><a href="#" id="advancedOptionsLink">Show advanced options&hellip;</a></td>
-           <td><button id="saveOptions" disabled="true">Save Options</button></td>
+           <td colspan="2"><a href="#" id="advancedOptionsLink">Show advanced options&hellip;</a></td>
         </tr>
         <tbody id='advancedOptions'>
           <tr>
             <td class="caption">Scroll step size</td>
             <td>
+                <div class="help">
+                  <div class="example">
+                    The size for basic movements (usually j/k/h/l).
+                  </div>
+                </div>
               <input id="scrollStepSize" type="number" />px
             </td>
           </tr>
@@ -315,7 +349,7 @@ unmapAll
             </td>
           </tr>
           <tr>
-            <td class="caption">Numbers used<br/> for filtered link hints</td>
+            <td class="caption">Numbers used<br/> for link hints</td>
             <td verticalAlign="top">
                 <div class="help">
                   <div class="example">
@@ -327,6 +361,15 @@ unmapAll
             </td>
           </tr>
           <tr>
+            <td class="caption" verticalAlign="top">Miscellaneous<br/>options</td>
+            <td verticalAlign="top" class="booleanOption">
+              <label>
+                <input id="smoothScroll" type="checkbox"/>
+                Use smooth scrolling
+              </label>
+            </td>
+          </tr>
+          <tr>
             <td class="caption"></td>
             <td verticalAlign="top" class="booleanOption">
               <div class="help">
@@ -364,7 +407,7 @@ unmapAll
               </div>
               <label>
                 <input id="regexFindMode" type="checkbox"/>
-                Treat find queries as regular expressions.
+                Treat find queries as regular expressions
               </label>
             </td>
           </tr>
@@ -433,14 +476,28 @@ unmapAll
           </tr>
         </tbody>
       </table>
+    </div>
 
-      <br/>
+    <!-- Some extra space which is hidden underneath the footer. -->
+    <div id="endSpace"/>
 
-      <footer id="showHelpDialogMessage">
-        Type <strong>?</strong> to show the Vimium help dialog.
-        <br/>
-        Type <strong>Ctrl-Enter</strong> in text inputs to save all options.
-      </footer>
+    <div id="footer">
+      <div id="footerWrapper">
+        <table id="footerTable">
+          <tr>
+            <td id="footerTableData">
+              <span id="helpText">
+                Type <strong>?</strong> to show the Vimium help dialog.
+                <br/>
+                Type <strong>Ctrl-Enter</strong> to save <i>all</i> options.
+              </span>
+            </td>
+            <td id="saveOptionsTableData">
+              <button id="saveOptions" disabled="true">No Changes</button>
+            </td>
+          </tr>
+        </table>
+      </div>
     </div>
   </body>
 </html>
diff --git a/pages/popup.html b/pages/popup.html
index 691414f2..775d6c07 100644
--- a/pages/popup.html
+++ b/pages/popup.html
@@ -76,7 +76,7 @@
       <div id="popupMenu">
         <ul>
           <li>
-             <span id="helpText">Type <strong>Ctrl-ENTER</strong> to save and close.</span>
+             <span id="helpText">Type <strong>Ctrl-Enter</strong> to save and close.</span>
              <a id="optionsLink" target="_blank">Options</a>
           </li>
         </ul>