From 37f5c0dd761df7abd20a9de1dbc5f19918cb27df Mon Sep 17 00:00:00 2001
From: Jason Miller <developit@users.noreply.github.com>
Date: Tue, 2 Oct 2018 15:53:21 +0200
Subject: [PATCH] Design review fixes (#172)

* Design review fixes

* Adding styles for mozjpeg & fixing some merge errors
---
 src/codecs/mozjpeg/options.tsx                | 22 ++---
 src/codecs/webp/options.tsx                   | 32 ++++---
 src/components/Options/index.tsx              | 38 ++++----
 src/components/Options/style.scss             | 95 +++++++++++++++----
 .../Output/custom-els/RangeInput/index.ts     |  3 +-
 .../custom-els/RangeInput/missing-types.d.ts  |  6 +-
 .../Output/custom-els/RangeInput/styles.css   | 29 ++++--
 7 files changed, 151 insertions(+), 74 deletions(-)

diff --git a/src/codecs/mozjpeg/options.tsx b/src/codecs/mozjpeg/options.tsx
index 91e0cf4f..beec46b3 100644
--- a/src/codecs/mozjpeg/options.tsx
+++ b/src/codecs/mozjpeg/options.tsx
@@ -1,6 +1,7 @@
 import { h, Component } from 'preact';
 import { bind, inputFieldChecked, inputFieldValueAsNumber } from '../../lib/util';
 import { EncodeOptions, MozJpegColorSpace } from './encoder';
+import '../../components/output/custom-els/RangeInput';
 
 type Props = {
   options: EncodeOptions,
@@ -40,9 +41,8 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
       <form>
         <label>
           Quality:
-          <input
+          <range-input
             name="quality"
-            type="range"
             min="0"
             max="100"
             value={'' + options.quality}
@@ -56,7 +56,7 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.baseline}
             onChange={this.onChange}
           />
-          Baseline (worse but legacy-compatible)
+          <span>Baseline (worse but legacy-compatible)</span>
         </label>
         <label style={{ display: options.baseline ? 'none' : '' }}>
           <input
@@ -65,7 +65,7 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.progressive}
             onChange={this.onChange}
           />
-          Progressive multi-pass rendering
+          <span>Progressive multi-pass rendering</span>
         </label>
         <label style={{ display: options.baseline ? '' : 'none' }}>
           <input
@@ -74,13 +74,12 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.optimize_coding}
             onChange={this.onChange}
           />
-          Optimize Huffman table
+          <span>Optimize Huffman table</span>
         </label>
         <label>
           Smoothing:
-          <input
+          <range-input
             name="smoothing"
-            type="range"
             min="0"
             max="100"
             value={'' + options.smoothing}
@@ -124,7 +123,7 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.trellis_multipass}
             onChange={this.onChange}
           />
-          Consider multiple scans during trellis quantization
+          <span>Consider multiple scans during trellis quantization</span>
         </label>
         <label style={{ display: options.trellis_multipass ? '' : 'none' }}>
           <input
@@ -133,7 +132,7 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.trellis_opt_zero}
             onChange={this.onChange}
           />
-          Optimize runs of zero blocks
+          <span>Optimize runs of zero blocks</span>
         </label>
         <label>
           <input
@@ -142,13 +141,12 @@ export default class MozJPEGEncoderOptions extends Component<Props, {}> {
             checked={options.trellis_opt_table}
             onChange={this.onChange}
           />
-          Optimize after trellis quantization
+          <span>Optimize after trellis quantization</span>
         </label>
         <label>
           Trellis quantization passes:
-          <input
+          <range-input
             name="trellis_loops"
-            type="range"
             min="1"
             max="50"
             value={'' + options.trellis_loops}
diff --git a/src/codecs/webp/options.tsx b/src/codecs/webp/options.tsx
index fbed0d90..7965f096 100644
--- a/src/codecs/webp/options.tsx
+++ b/src/codecs/webp/options.tsx
@@ -2,6 +2,7 @@ import { h, Component } from 'preact';
 import { bind, inputFieldCheckedAsNumber, inputFieldValueAsNumber } from '../../lib/util';
 import { EncodeOptions, WebPImageHint } from './encoder';
 import * as styles from './styles.scss';
+import '../../components/output/custom-els/RangeInput';
 
 type Props = {
   options: EncodeOptions,
@@ -105,7 +106,7 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             value={'' + WebPImageHint.WEBP_HINT_GRAPH}
             onChange={this.onChange}
           />
-          Discrete tone image (graph, map-tile etc)
+          <span>Discrete tone image (graph, map-tile etc)</span>
         </label>
       </div>
     );
@@ -135,6 +136,7 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             onChange={this.onChange}
           />
         </label>
+        <hr />
         <label>
           <input
             name="alpha_compression"
@@ -164,16 +166,7 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             onChange={this.onChange}
           />
         </label>
-        <label>
-          Spacial noise shaping:
-          <range-input
-            name="sns_strength"
-            min="0"
-            max="100"
-            value={'' + options.sns_strength}
-            onChange={this.onChange}
-          />
-        </label>
+        <hr />
         <label>
           <input
             name="autofilter"
@@ -181,7 +174,7 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             checked={!!options.autofilter}
             onChange={this.onChange}
           />
-          Auto adjust filter strength
+          <span>Auto adjust filter strength</span>
         </label>
         <label>
           Filter strength:
@@ -222,6 +215,7 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
           />
           Sharp RGB->YUV conversion
         </label>
+        <hr />
         <label>
           Passes:
           <range-input
@@ -232,6 +226,16 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             onChange={this.onChange}
           />
         </label>
+        <label>
+          Spacial noise shaping:
+          <range-input
+            name="sns_strength"
+            min="0"
+            max="100"
+            value={'' + options.sns_strength}
+            onChange={this.onChange}
+          />
+        </label>
         <label>
           Preprocessing type:
           <select
@@ -295,7 +299,9 @@ export default class WebPEncoderOptions extends Component<Props, {}> {
             checked={!!options.exact}
             onChange={this.onChange}
           />
-          Preserve transparent data. Otherwise, pixels with zero alpha will have RGB also zeroed.
+          <span>
+            Preserve transparent data. Otherwise, pixels with zero alpha will have RGB also zeroed.
+          </span>
         </label>
       </form>
     );
diff --git a/src/components/Options/index.tsx b/src/components/Options/index.tsx
index 505301ec..cd8ec23c 100644
--- a/src/components/Options/index.tsx
+++ b/src/components/Options/index.tsx
@@ -25,13 +25,13 @@ import * as browserJP2 from '../../codecs/browser-jp2/encoder';
 import * as browserBMP from '../../codecs/browser-bmp/encoder';
 import * as browserPDF from '../../codecs/browser-pdf/encoder';
 import {
-    EncoderState,
-    EncoderType,
-    EncoderOptions,
-    encoders,
-    encodersSupported,
-    EncoderSupportMap,
-    encoderMap,
+  EncoderState,
+  EncoderType,
+  EncoderOptions,
+  encoders,
+  encodersSupported,
+  EncoderSupportMap,
+  encoderMap,
 } from '../../codecs/encoders';
 import { QuantizeOptions } from '../../codecs/imagequant/quantizer';
 import { ResizeOptions } from '../../codecs/resize/resize';
@@ -150,20 +150,9 @@ export default class Options extends Component<Props, State> {
           {titles[orientation][imageIndex]}
           {', '}
           {encoderMap[encoderState.type].label}
-
-          {(downloadUrl && imageFile) && (
-            <a
-              class={style.download}
-              href={downloadUrl}
-              download={imageFile.name}
-              title="Download"
-            >
-              <DownloadIcon />
-            </a>
-          )}
         </h2>
 
-        <div class={style.inner}>
+        <div class={style.content}>
           <section class={style.picker}>
             {encoderSupportMap ?
               <select value={encoderState.type} onChange={this.onEncoderTypeChange}>
@@ -236,6 +225,17 @@ export default class Options extends Component<Props, State> {
             file={imageFile}
             compareTo={imageFile === sourceImageFile ? undefined : sourceImageFile}
           />
+
+          {(downloadUrl && imageFile) && (
+            <a
+              class={style.download}
+              href={downloadUrl}
+              download={imageFile.name}
+              title="Download"
+            >
+              <DownloadIcon />
+            </a>
+          )}
         </div>
       </div>
     );
diff --git a/src/components/Options/style.scss b/src/components/Options/style.scss
index 3134217f..4948a06d 100644
--- a/src/components/Options/style.scss
+++ b/src/components/Options/style.scss
@@ -49,24 +49,82 @@ Note: These styles are temporary. They will be replaced before going live.
     }
   }
 
+  .content {
+    max-height: calc(75vh - 100px);
+    overflow: auto;
+    touch-action: pan-y;
+    -webkit-overflow-scrolling: touch;
+  }
+
+  .picker {
+    margin: 5px 15px;
+
+    select {
+      display: block;
+      width: 100%;
+      box-sizing: border-box;
+      -webkit-appearance: none;
+      appearance: none;
+      padding: 10px 30px 10px 10px;
+      background: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="25" height="5"><polygon fill="#fff" points="10,0 5,5 0,0"/></svg>') right center no-repeat;
+      background-color: var(--gray-dark);
+      opacity: 0.9;
+      border: none;
+      font: inherit;
+      color: white;
+      transition: box-shadow 150ms ease;
+
+      &:hover {
+        opacity: 1;
+      }
+      &:focus {
+        opacity: 1;
+        outline: none;
+        box-shadow: 0 0 0 2px var(--button-fg, #ccc);
+      }
+    }
+  }
+
+  .title {
+    display: flex;
+    align-items: center;
+    padding: 10px 15px;
+    margin: 0 0 12px;
+    background: rgba(0,0,0,0.9);
+    font: inherit;
+  }
+
   label {
     display: block;
-    @extend .row;
+    padding: 5px;
+    margin: 0 10px;
+    display: flex;
+    flex-wrap: wrap;
 
-    input {
-      vertical-align: middle;
+    // prevent labels from wrapping below checkboxes
+    > span {
+      flex: 1;
     }
 
     input[type=checkbox],
     input[type=radio] {
-      margin-right: 8px;
+      flex: 0;
+      margin: 2px 8px 0 0;
     }
 
     range-input {
       display: block;
-      width: 90%;
+      flex: 1 0 100%;
+      margin: 2px 0;
     }
   }
+
+  hr {
+    height: 1px;
+    border: none;
+    margin: 5px 0;
+    box-shadow: inset 0 0.5px 0 rgba(0, 0, 0, 0.4), inset 0 -0.5px 0 rgba(255, 255, 255, 0.2);
+  }
 }
 
 .picker {
@@ -85,31 +143,27 @@ Note: These styles are temporary. They will be replaced before going live.
     border: none;
     font: inherit;
     color: white;
+  }
 
-    &:hover {
-      opacity: 1;
-    }
-    &:focus {
-      opacity: 1;
-      outline: none;
-      box-shadow: 0 0 0 2px var(--button-fg, #ccc);
-    }
+  hr {
+    height: 1px;
+    border: none;
+    margin: 5px 0;
+    box-shadow: inset 0 0.5px 0 rgba(0, 0, 0, 0.4), inset 0 -0.5px 0 rgba(255, 255, 255, 0.2);
   }
 }
 
-.title {
+.size-details {
   display: flex;
   align-items: center;
-  padding: 10px 15px;
-  margin: 0 0 12px;
-  background: rgba(0,0,0,0.9);
-  font: inherit;
+  padding: 5px 15px;
+  background: rgba(0,0,0,0.5);
 }
 
 .download {
   flex: 0;
   margin: 0 0 0 auto;
-  background: rgba(0,0,0,0.7);
+  background: rgba(255,255,255,0.1);
   border-radius: 50%;
   padding: 5px;
   width: 16px;
@@ -159,7 +213,7 @@ Note: These styles are temporary. They will be replaced before going live.
 .preprocessors {
   padding: 5px 0;
   margin: 5px 0;
-  box-shadow: inset 0 -.5px 0 rgba(0,0,0,0.25), 0 .5px 0 rgba(255,255,255,0.15);
+  box-shadow: inset 0 -.5px 0 rgba(0,0,0,0.4), 0 .5px 0 rgba(255,255,255,0.2);
 }
 
 .toggle {
@@ -167,4 +221,5 @@ Note: These styles are temporary. They will be replaced before going live.
   position: relative;
   align-content: center;
   font-size: 14px;
+  box-shadow: inset 0 -.5px 0 rgba(0,0,0,0.4), 0 .5px 0 rgba(255,255,255,0.2);
 }
diff --git a/src/components/Output/custom-els/RangeInput/index.ts b/src/components/Output/custom-els/RangeInput/index.ts
index bf5f522a..39cbd38f 100644
--- a/src/components/Output/custom-els/RangeInput/index.ts
+++ b/src/components/Output/custom-els/RangeInput/index.ts
@@ -73,8 +73,9 @@ class RangeInputElement extends HTMLElement {
     const displayValue = labelPrecision ? value.toPrecision(labelPrecision) :
       Math.round(value).toString();
 
-    this.style.setProperty('--value-percent', percent + '%');
     this._valueDisplay.textContent = displayValue;
+    this.style.setProperty('--value-percent', percent + '%');
+    this.style.setProperty('--value-width', '' + displayValue.length);
   }
 
   private _reflectAttributes() {
diff --git a/src/components/Output/custom-els/RangeInput/missing-types.d.ts b/src/components/Output/custom-els/RangeInput/missing-types.d.ts
index d566207d..41c48e40 100644
--- a/src/components/Output/custom-els/RangeInput/missing-types.d.ts
+++ b/src/components/Output/custom-els/RangeInput/missing-types.d.ts
@@ -1,5 +1,9 @@
 declare namespace JSX {
+  interface RangeInputAttributes extends HTMLAttributes {
+    reversed?: boolean;
+  }
+
   interface IntrinsicElements {
-    'range-input': HTMLAttributes;
+    'range-input': RangeInputAttributes;
   }
 }
diff --git a/src/components/Output/custom-els/RangeInput/styles.css b/src/components/Output/custom-els/RangeInput/styles.css
index d3512704..c0397d80 100644
--- a/src/components/Output/custom-els/RangeInput/styles.css
+++ b/src/components/Output/custom-els/RangeInput/styles.css
@@ -9,9 +9,29 @@ range-input {
   overflow: visible;
 }
 
+/* Disabled inputs are greyed out */
+range-input[disabled] {
+  filter: grayscale(1);
+}
+
+/* Reversed Variant */
+range-input[reversed] input,
+range-input[reversed]::before,
+range-input[reversed] > div {
+  transform: scaleX(-1);
+}
+range-input[reversed] > div > span {
+  transform: scaleX(-1) scale(.2);
+}
+range-input[reversed] input:focus + div span {
+  transform: scaleX(-1) scale(1);
+}
+
+
 range-input input {
   position: relative;
   flex: 1;
+  vertical-align: middle;
   width: 100%;
   padding: 0 !important;
   margin: 0 !important;
@@ -20,7 +40,6 @@ range-input input {
   -webkit-appearance: none;
   outline: none;
 }
-
 range-input input::-webkit-slider-runnable-track,
 range-input input::-moz-range-track,
 range-input input::-ms-track {
@@ -45,9 +64,6 @@ range-input::before {
   box-shadow: 0 -.5px 0 rgba(0,0,0,0.3), inset 0 .5px 0 rgba(255,255,255,0.2), 0 .5px 0 rgba(255,255,255,0.3);
   background: linear-gradient(#34B9EB, #218ab1) 0/ var(--value-percent, 0%) 100% no-repeat #eee;
 }
-range-input[disabled]::before {
-  background: linear-gradient(#aaa, #888) 0/ var(--value-percent, 0%) 100% no-repeat #eee;
-}
 
 range-input input::-webkit-slider-thumb {
   appearance: none;
@@ -62,10 +78,6 @@ range-input input::-webkit-slider-thumb {
   outline: none;
 }
 
-range-input[disabled] input::-webkit-slider-thumb {
-  background: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><circle cx="5" cy="5" r="1" fill="#666" /></svg>') center no-repeat #999;
-}
-
 range-input input:focus::-webkit-slider-thumb {
   box-shadow: 0 1px 3px rgba(0,0,0,0.5);
 }
@@ -96,6 +108,7 @@ range-input > div > span {
   transform: scale(.2);
   color: #fff;
   font: inherit;
+  font-size: calc(100% - var(--value-width, 3) / 5 * .2em);
   text-overflow: clip;
   text-shadow: 0 -.5px 0 rgba(0,0,0,0.4);
   transition: transform 200ms ease, opacity 200ms ease;