Supersize: Adjusts infocard and options

Changed infocard so that focusing on a node will lock the infocard. Focus can be removed by pressing the escape key. The options have been reorganized so that regular expressions are now higher in the list. Additionally added a "?" key shortcut to open help, fixed the toolbar on small screen sizes, and added input validation for the regex inputs. Bug: 847599 Change-Id: I124afe3bdc162b55889b25ba35b403aca72a8435 Reviewed-on: https://chromium-review.googlesource.com/1145230 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Tiger Oakes <tigero@google.com> Cr-Commit-Position: refs/heads/master@{#576933}
marcosholgado · Jul 20, 2018 · 318233c · 318233c
1 parent 799270f
commit 318233c
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 68 deletions.
diff --git a/tools/binary_size/libsupersize/html_report.py b/tools/binary_size/libsupersize/html_report.py
@@ -118,6 +118,9 @@ def _MakeTreeViewList(symbols, include_all_symbols):
   main_symbols = dex_symbols + ordered_symbols[:symbol_count]
   extra_symbols = ordered_symbols[symbol_count:]
 
+  logging.info('Found %d large symbols, %s small symbols',
+               len(main_symbols), len(extra_symbols))
+
   # Bundle symbols by the file they belong to,
   # and add all the file buckets into file_nodes
   for symbol in main_symbols:

diff --git a/tools/binary_size/libsupersize/static/index.html b/tools/binary_size/libsupersize/static/index.html
@@ -94,6 +94,7 @@
         }
         .subhead,
         .subhead-2 {
+            margin-bottom: 0.5em;
             font-family: "Google Sans", Arial, sans-serif;
             font-weight: 500;
             font-size: 16px;
@@ -199,7 +200,7 @@
         }
 
         .diff .size-header::after {
-            content: ' diff';
+            content: " diff";
         }
     </style>
     <link rel="stylesheet" href="options.css">
@@ -224,9 +225,9 @@ <h1 class="headline">Binary Size Analysis</h1>
                     -->
                     <path d="M6 2a2 2 0 0 0-2 2v16c0 1.1.9 2 2 2h12a2 2 0 0 0 2-2V8l-6-6H6zm0 2h7v5h5v11H6V4zm6 7l-4 4h3v4h2v-4h3l-4-4z" />
                 </svg>
-                Upload data
+                <span class="label-text">Upload data</span>
             </label>
-            <a href="https://chromium.googlesource.com/chromium/src/+/master/tools/binary_size/html_report_faq.md" class="icon-button" title="FAQ">
+            <a href="https://chromium.googlesource.com/chromium/src/+/master/tools/binary_size/html_report_faq.md" class="icon-button" title="FAQ" id="faq">
                 <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="#5f6368">
                     <path d="M11,18h2v-2h-2V18z M12,2C6.48,2,2,6.48,2,12s4.48,10,10,10s10-4.48,10-10S17.52,2,12,2z M12,20c-4.41,0-8-3.59-8-8
                             s3.59-8,8-8s8,3.59,8,8S16.41,20,12,20z M12,6c-2.21,0-4,1.79-4,4h2c0-1.1,0.9-2,2-2s2,0.9,2,2c0,2-3,1.75-3,5h2c0-2.25,3-2.5,3-5
@@ -266,20 +267,28 @@ <h1 class="headline">Binary Size Analysis</h1>
             </button>
         </header>
 
-        <h2 class="subhead">Size options</h2>
-        <p class="select-wrapper">
-            <select id="byteunit" name="byteunit">
-                <option value="B">B - bytes</option>
-                <option value="KiB">KiB - kilibytes</option>
-                <option value="MiB" selected>MiB - megabytes</option>
-                <option value="GiB">GiB - gigabytes</option>
-            </select>
-            <label class="select-label" for="byteunit">Byte unit</label>
-        </p>
-        <p class="checkbox-wrapper">
-            <input type="checkbox" id="methodcount" name="method_count" value="on">
-            <label class="checkbox-label" for="methodcount">Show dex method count rather than size</label>
-        </p>
+        <fieldset>
+            <legend class="subhead">Size options</legend>
+            <div class="checkbox-wrapper">
+                <input type="checkbox" id="methodcount" name="method_count" value="on">
+                <label class="checkbox-label" for="methodcount">Show dex method count rather than size</label>
+            </div>
+            <p class="select-wrapper">
+                <select id="byteunit" name="byteunit">
+                    <option value="B">B - bytes</option>
+                    <option value="KiB">KiB - kilibytes</option>
+                    <option value="MiB" selected>MiB - megabytes</option>
+                    <option value="GiB">GiB - gigabytes</option>
+                </select>
+                <label class="select-label" for="byteunit">Byte unit</label>
+            </p>
+            <p class="input-wrapper">
+                <input type="number" id="minsize" name="min_size" value="0" min="0">
+                <label class="input-label" for="minsize">
+                    Minimum size (bytes)
+                </label>
+            </p>
+        </fieldset>
 
         <fieldset>
             <legend class="subhead">Group symbols by</legend>
@@ -293,6 +302,24 @@ <h2 class="subhead">Size options</h2>
             </div>
         </fieldset>
 
+        <fieldset>
+            <legend class="subhead">Search by regex</legend>
+            <div class="input-wrapper">
+                <input class="input-regex" type="text" id="includeregex" name="include" placeholder=".+@.+" aria-describedby="include-error">
+                <label class="input-label" for="includeregex">
+                    Symbols must contain
+                </label>
+                <p class="input-error" id="include-error"></p>
+            </div>
+            <div class="input-wrapper">
+                <input class="input-regex" type="text" id="excluderegex" name="exclude" placeholder="\(.+\)"  aria-describedby="exclude-error">
+                <label class="input-label" for="excluderegex">
+                        Symbols must exclude
+                </label>
+                <p class="input-error" id="exclude-error"></p>
+            </div>
+        </fieldset>
+
         <fieldset id="types-filter">
             <legend class="subhead">Symbol types to show</legend>
             <div class="checkbox-wrapper">
@@ -364,31 +391,6 @@ <h2 class="subhead">Size options</h2>
             <button type="button" class="text-button" id="type-all">Select all</button>
             <button type="button" class="text-button" id="type-none">Select none</button>
         </fieldset>
-
-        <fieldset>
-            <legend class="subhead">Advanced filters</legend>
-            <p class="input-wrapper">
-                <input type="number" id="minsize" name="min_size" value="0" min="0">
-                <label class="input-label" for="minsize">
-                    Minimum size (bytes)
-                </label>
-            </p>
-            <fieldset>
-                <legend class="subhead-2">Regular expressions</legend>
-                <div class="input-wrapper">
-                    <input class="input-regex" type="text" id="includeregex" name="include" placeholder=".+@.+">
-                    <label class="input-label" for="includeregex">
-                        Symbols must contain
-                    </label>
-                </div>
-                <div class="input-wrapper">
-                    <input class="input-regex" type="text" id="excluderegex" name="exclude" placeholder="\(.+\)">
-                    <label class="input-label" for="excluderegex">
-                            Symbols must exclude
-                    </label>
-                </div>
-            </fieldset>
-        </fieldset>
     </form>
     <div class="symbols">
         <div hidden id="icons">
@@ -397,7 +399,7 @@ <h2 class="subhead">Size options</h2>
                 <path d="M9.17,6l2,2H20v10L4,18V6H9.17 M10,4H4C2.9,4,2.01,4.9,2.01,6L2,18c0,1.1,0.9,2,2,2h16c1.1,0,2-0.9,2-2V8c0-1.1-0.9-2-2-2
                             h-8L10,4L10,4z" />
             </svg>
-            <svg class='icon componenticon' height='24' width='24' fill='#5f6368'>
+            <svg class="icon componenticon" height="24" width="24" fill="#5f6368">
                 <title>Component</title>
                 <path d="M9,13.75c-2.34,0-7,1.17-7,3.5V19h14v-1.75C16,14.92,11.34,13.75,9,13.75z M4.34,17c0.84-0.58,2.87-1.25,4.66-1.25
                          s3.82,0.67,4.66,1.25H4.34z" />

diff --git a/tools/binary_size/libsupersize/static/infocard.css b/tools/binary_size/libsupersize/static/infocard.css
@@ -20,7 +20,8 @@
   transition: 0.3s ease transform, 0.3s ease opacity, 0.3s ease visibility;
 }
 .tree-container:hover ~ .infocards,
-.tree-container.focused ~ .infocards {
+.tree-container.focused ~ .infocards,
+.infocards:hover {
   visibility: visible;
   opacity: 1;
   transform: none;

diff --git a/tools/binary_size/libsupersize/static/options.css b/tools/binary_size/libsupersize/static/options.css
@@ -43,7 +43,7 @@
 fieldset {
   border: 0;
   padding: 0;
-  margin: 2em 0 1em;
+  margin: 1em 0;
 }
 .options fieldset:first-of-type {
   margin-top: 1em;
@@ -83,7 +83,7 @@ legend {
 
 .text-button {
   padding: 0 8px;
-  line-height: 36px;
+  height: 36px;
   border-radius: 4px;
   color: #1a73e8;
   font-family: 'Google Sans', Arial, sans-serif;
@@ -136,8 +136,6 @@ input:focus + label.filled-button:active {
   position: relative;
   border-top-left-radius: 4px;
   border-top-right-radius: 4px;
-  background: rgba(0, 0, 0, 0.04);
-  margin-bottom: 1em;
 }
 input[type='text'],
 input[type='number'],
@@ -158,14 +156,15 @@ select {
   position: absolute;
   z-index: -1;
   top: 0;
-  bottom: -1px;
   left: 0;
   right: 0;
-  font-size: 12px;
+  height: 52px;
   line-height: 20px;
   padding: 0 12px;
+  font-size: 12px;
   color: #5f6368;
   border-bottom: 1px solid currentColor;
+  background: rgba(0, 0, 0, 0.04);
 }
 .select-label::after {
   content: '';
@@ -189,6 +188,26 @@ select:focus + .select-label::after {
   transform: rotate(180deg);
 }
 
+.input-error {
+  margin: 4px 0;
+  min-height: 2.5em;
+  opacity: 0;
+  color: #d93025;
+  font-size: 12px;
+}
+.input-regex:invalid,
+.input-regex[aria-invalid='true'] {
+  caret-color: #d93025;
+}
+.input-regex:invalid + .input-label,
+.input-regex[aria-invalid='true'] + .input-label {
+  color: #d93025;
+}
+.input-regex:invalid ~ .input-error,
+.input-regex[aria-invalid='true'] ~ .input-error {
+  opacity: 1;
+}
+
 /** <input type='checkbox' or 'radio'> elements */
 input[type='checkbox'],
 input[type='radio'] {
@@ -269,6 +288,8 @@ input[type='radio']:focus + .radio-label {
 }
 
 input[type='file'] {
+  position: absolute;
+  top: -100%;
   opacity: 0;
 }
 
@@ -280,12 +301,20 @@ input[type='file'] {
   .show-options .scrim {
     display: block;
   }
-  .appbar,
+
+  .appbar-inner,
   .symbols {
     padding: 0 16px;
   }
   .appbar-progress {
     margin: 0 -16px;
     width: calc(100% + 32px);
   }
+
+  .filled-button {
+    padding-right: 6px;
+  }
+  .filled-button .label-text {
+    display: none;
+  }
 }
diff --git a/tools/binary_size/libsupersize/static/state.js b/tools/binary_size/libsupersize/static/state.js
@@ -242,6 +242,29 @@ function _startListeners() {
   setMethodCountModeUI();
   methodCountInput.addEventListener('change', setMethodCountModeUI);
 
+  /**
+   * Display error text on blur for regex inputs, if the input is invalid.
+   * @param {Event} event
+   */
+  function checkForRegExError(event) {
+    const input = /** @type {HTMLInputElement} */ (event.currentTarget);
+    const errorBox = document.getElementById(
+      input.getAttribute('aria-describedby')
+    );
+    try {
+      new RegExp(input.value);
+      errorBox.textContent = '';
+      input.setAttribute('aria-invalid', 'false');
+    } catch (err) {
+      errorBox.textContent = err.message;
+      input.setAttribute('aria-invalid', 'true');
+    }
+  }
+  for (const input of document.getElementsByClassName('input-regex')) {
+    input.addEventListener('blur', checkForRegExError);
+    input.dispatchEvent(new Event('blur'));
+  }
+
   document.getElementById('type-all').addEventListener('click', () => {
     for (const checkbox of typeCheckboxes) {
       checkbox.checked = true;
@@ -335,7 +358,7 @@ function _makeSizeTextGetter() {
   function getSizeContents(node) {
     if (state.has('method_count')) {
       const {count: methodCount = 0} =
-          node.childStats[_DEX_METHOD_SYMBOL_TYPE] || {};
+        node.childStats[_DEX_METHOD_SYMBOL_TYPE] || {};
       const methodStr = methodCount.toLocaleString(_LOCALE, {
         useGrouping: true,
       });

diff --git a/tools/binary_size/libsupersize/static/tree-ui.js b/tools/binary_size/libsupersize/static/tree-ui.js
@@ -11,7 +11,7 @@
  * Binary Size Analysis HTML report.
  */
 
-{
+const newTreeElement = (() => {
   /** Capture one of: "::", "../", "./", "/", "#" */
   const _SPECIAL_CHAR_REGEX = /(::|(?:\.*\/)+|#)/g;
   /** Insert zero-width space after capture group */
@@ -107,7 +107,7 @@
   }
 
   /**
-   * Keydown event handler to move focus for the given element
+   * Tree view keydown event handler to move focus for the given element.
    * @param {KeyboardEvent} event
    */
   function _handleKeyNavigation(event) {
@@ -163,11 +163,10 @@
         break;
       // If closed tree, open tree. Otherwise, move to first child.
       case 'ArrowRight': {
-        const data = _uiNodeData.get(link);
-        if (!data.children || data.children.length !== 0) {
-          const isExpanded =
-            link.parentElement.getAttribute('aria-expanded') === 'true';
-          if (isExpanded) {
+        const expanded = link.parentElement.getAttribute('aria-expanded');
+        if (expanded != null) {
+          // Leafs do not have the aria-expanded property
+          if (expanded === 'true') {
             _focusNext();
           } else {
             _toggle();
@@ -213,6 +212,10 @@
           }
         }
         break;
+      // Remove focus from the tree view.
+      case 'Escape':
+        link.blur();
+        break;
       // If a letter was pressed, find a node starting with that character.
       default:
         if (event.key.length === 1 && event.key.match(/\S/)) {
@@ -227,6 +230,18 @@
     }
   }
 
+  /**
+   * Display the infocard when a node is hovered over, unless a node is
+   * currently focused.
+   * @param {MouseEvent} event
+   */
+  function _handleMouseOver(event) {
+    const active = document.activeElement;
+    if (!active || !active.classList.contains('node')) {
+      displayInfocard(_uiNodeData.get(event.currentTarget));
+    }
+  }
+
   /**
    * Replace the contents of the size element for a tree node.
    * @param {HTMLElement} sizeElement Element that should display the size
@@ -288,9 +303,7 @@
     // Set the byte size and hover text
     _setSize(element.querySelector('.size'), data);
 
-    link.addEventListener('mouseover', event =>
-      displayInfocard(_uiNodeData.get(event.currentTarget))
-    );
+    link.addEventListener('mouseover', _handleMouseOver);
     if (!isLeaf) {
       link.addEventListener('click', _toggleTreeElement);
     }
@@ -317,9 +330,15 @@
   _symbolTree.addEventListener('focusout', event =>
     event.currentTarget.parentElement.classList.remove('focused')
   );
+  window.addEventListener('keydown', event => {
+    if (event.key === '?' && event.target.tagName !== 'INPUT') {
+      // Open help when "?" is pressed
+      document.getElementById('faq').click();
+    }
+  });
 
-  self.newTreeElement = newTreeElement;
-}
+  return newTreeElement
+})();
 
 {
   class ProgressBar {