linter and compact layout improvements (#749)

* linter and compact layout improvements

Closes #748

While investigating the best way to fix linter scrolling, when I double-checked the compact layout, an old bug I meant to fix a long time ago was immediately apparent. Basically, the linter adds/removes errors as you type, causing the editor to bounce up and down, making it practically unusable. 

This PR fixes scrolling, and also collapses options and the linter in the compact layout, but always shows the collapsed linter so you're aware of the error count without the content jumping. It also collapses options in the non-compact layout if the viewport is too short to accommodate them, factoring in the min-height of the linter. All automatic collapsing factors in whether a linter is active so they can adjust accordingly, and disables the setting of collapsed state prefs, since we're deciding the pref anyway, and this allows for re-expanding on resize based on the previous pref.

It's quite possible I failed to account for certain scenarios, so try to break it. Also think it's problematic for the linter to not always be visible if enabled, so I hooked up a 40px fixed header on scroll with just the linter in it for the compact layout.

A few other little details are included. I removed redundant line and column numbers spelled out at the end of lint messages to prevent horizontal overflow. I noticed that the expand/collapse prefs do not toggle correctly when clicking directly on the details-marker arrow. Simplest solution was covering them with the `h2` (we may wanna hook up the manager as well). Also, unrelated, but I switched to opacity to hide resizing sectioned editors, because `visibility: hidden;` breaks editor auto-focus.

If either of you guys wanna fix any bugs, or improve any code, feel free to just commit to this PR directly.

* linter and compact layout improvements

* linter and compact layout improvements

* No usercss scroll listener and delay header check

* Some code tweaks

edit.html

@@ -429,7 +429,7 @@
           </div>
         </div>
       </details>
-      <details id="lint" class="hidden" data-pref="editor.lint.expanded">
+      <details id="lint" class="hidden-unless-compact" data-pref="editor.lint.expanded">
         <summary>
           <h2 i18n-text="linterIssues">: <span id="issue-count"></span>
             <a id="lint-help" href="#" class="svg-inline-wrapper intercepts-click" tabindex="0">
@@ -437,7 +437,9 @@
             </a>
           </h2>
         </summary>
-        <div class="lint-report-container"></div>
+        <div class="lint-scroll-container">
+          <div class="lint-report-container"></div>
+        </div>
       </details>
       <div id="footer" class="hidden">
         <a href="https://github.com/openstyles/stylus/wiki/Usercss"

edit/edit.css

@@ -195,6 +195,8 @@ input:invalid {
   border-bottom: 1px dotted transparent;
   margin-top: .1em;
   margin-bottom: .1em;
+  margin-left: -13px;
+  padding-left: 13px; /* clicking directly on details-marker doesn't set pref so we cover it with h2 */
 }
 
 #header summary:hover h2 {
@@ -272,7 +274,7 @@ input:invalid {
   border-top: 2px solid hsl(0, 0%, 80%);
 }
 .section-editor:not(.section-editor-ready) .section {
-  visibility: hidden;
+  opacity: 0 !important;
 }
 .section-editor:not(.section-editor-ready) .CodeMirror {
   height: 0;
@@ -654,21 +656,48 @@ body:not(.find-open) [data-match-highlight-count="1"] .CodeMirror-selection-high
 
 /************ lint ************/
 #lint {
+  overflow: hidden;
+  margin: .5rem -1rem 0;
+  min-height: 30px;
+  padding: 0;
+  box-sizing: border-box;
+  display: flex;
+  flex-grow: 1;
+  position: relative;
+}
+#lint > summary {
+  position: relative;
+  margin-left: 0;
+  padding-left: 4px;
+}
+#lint[open]:not(.hidden-unless-compact) {
+  min-height: 130px;
+}
+#lint summary h2 {
+  margin-left: -16px;
+}
+#lint > .lint-scroll-container {
+  margin: 42px 1rem 0;
+  position: absolute;
+  top: 0;
+  bottom: 0;
+  left: 0;
+  right: 0;
   overflow-y: auto;
   overflow-x: hidden;
-  }
-#lint > summary {
-  /* workaround for overflow:auto to show the toggle triangle */
-  position: absolute;
-}
-#lint > div {
-  margin-top: 2.75rem;
 }
 #lint table {
   font-size: 100%;
   border-spacing: 0;
   margin-bottom: 1rem;
   line-height: 1.0;
+  width: 100%;
+}
+#lint tr td:last-child {
+  width: 100%;
+}
+#lint td[role="line"] {
+  padding-left: 0.25rem;
 }
 #lint table:last-child {
   margin-bottom: 0;
@@ -679,6 +708,7 @@ body:not(.find-open) [data-match-highlight-count="1"] .CodeMirror-selection-high
 #lint caption {
   text-align: left;
   font-weight: bold;
+  padding-bottom: 6px;
 }
 #lint tbody {
   font-size: 85%;
@@ -759,13 +789,13 @@ html:not(.usercss) .usercss-only,
   display: none !important; /* hide during page init */
 }
 
-#lint {
+body:not(.compact-layout) .hidden-unless-compact,
-  padding: 1rem 0 0;
+body.linter-disabled .hidden-unless-compact {
-  box-sizing: border-box;
+  display: none !important;
 }
 
 #options:not([open]) + #lint {
-  padding-top: 0;
+  margin-top: 0;
 }
 
 #options-wrapper .options-column:nth-child(2) {
@@ -846,6 +876,21 @@ html:not(.usercss) .usercss-only,
     border-bottom: 1px dashed #AAA;
     padding: 0;
   }
+  .fixed-header {
+    padding-top: 40px;
+  }
+  .fixed-header #header {
+    min-height: 40px;
+    position: fixed;
+    top: 0;
+    left: 0;
+    right: 0;
+    padding: 8px 0 0;
+    background-color: #fff;
+  }
+  .fixed-header #header > *:not(#lint) {
+    display: none !important;
+  }
   #actions {
     display: flex;
     flex-wrap: wrap;
@@ -898,17 +943,10 @@ html:not(.usercss) .usercss-only,
   #options:not([open]) + #lint:not([open]) {
     margin-top: 0;
   }
-  #lint {
-    overflow: initial;
-  }
   #lint summary {
     position: static;
     margin-bottom:  0;
   }
-  #lint tbody {
-    display: flex;
-    flex-direction: column;
-  }
   #options summary {
     margin-left: 0;
     padding-left: 4px;
@@ -927,25 +965,17 @@ html:not(.usercss) .usercss-only,
     position: relative;
     top: 0.2rem;
   }
-  #lint > div {
+  #lint > .lint-scroll-container {
-    margin-top: 0;
+    margin: 32px 1rem 0;
-    overflow: hidden;
+    bottom: 6px;
   }
   #lint {
-    padding: 0 1rem .5rem;
+    padding: 0;
+    margin: 1rem 0 0;
   }
   #lint > summary {
     margin-top: 0;
   }
-  #lint caption {
-    text-indent: 4px;
-  }
-  #lint table {
-    width: 100%;
-  }
-  #lint td[role="message"] {
-    max-width: none;
-  }
   #lint:not([open]) + #footer {
     margin: .25em 0 -1em .25em;
   }

edit/edit.js

@@ -22,6 +22,8 @@ const CssToProperty = Object.entries(propertyToCss)
 
 let editor;
 
+let scrollPointTimer;
+
 document.addEventListener('visibilitychange', beforeUnload);
 window.addEventListener('beforeunload', beforeUnload);
 msg.onExtension(onRuntimeMessage);
@@ -172,8 +174,11 @@ preinit();
         $('#preview-label').classList.toggle('hidden', !style.id);
 
         $('#beautify').onclick = () => beautify(editor.getEditors());
-        $('#lint').addEventListener('scroll', hideLintHeaderOnScroll, {passive: true});
+        window.addEventListener('resize', () => {
-        window.addEventListener('resize', () => debounce(rememberWindowSize, 100));
+          debounce(rememberWindowSize, 100);
+          detectLayout();
+        });
+        detectLayout();
         editor = (usercss ? createSourceEditor : createSectionsEditor)({
           style,
           onTitleChanged: updateTitle
@@ -476,15 +481,6 @@ function showCodeMirrorPopup(title, html, options) {
   return popup;
 }
 
-function hideLintHeaderOnScroll() {
-  // workaround part2 for the <details> not showing its toggle icon: hide <summary> on scroll
-  const newOpacity = this.scrollTop === 0 ? '' : '0';
-  const style = this.firstElementChild.style;
-  if (style.opacity !== newOpacity) {
-    style.opacity = newOpacity;
-  }
-}
-
 function rememberWindowSize() {
   if (
     document.visibilityState === 'visible' &&
@@ -500,6 +496,67 @@ function rememberWindowSize() {
   }
 }
 
+prefs.subscribe(['editor.linter'], (key, value) => {
+  $('body').classList.toggle('linter-disabled', value === '');
+});
+
+function fixedHeader() {
+  const scrollPoint = $('#header').clientHeight - 40;
+  const linterEnabled = prefs.get('editor.linter') !== '';
+  if (window.scrollY >= scrollPoint && !$('.fixed-header') && linterEnabled) {
+    $('body').classList.add('fixed-header');
+  } else if (window.scrollY < 40 && linterEnabled) {
+    $('body').classList.remove('fixed-header');
+  }
+}
+
+function detectLayout() {
+  const body = $('body');
+  const options = $('#options');
+  const lint = $('#lint');
+  const compact = window.innerWidth <= 850;
+  const shortViewportLinter = window.innerHeight < 692;
+  const shortViewportNoLinter = window.innerHeight < 554;
+  const linterEnabled = prefs.get('editor.linter') !== '';
+  if (compact) {
+    body.classList.add('compact-layout');
+    options.removeAttribute('open');
+    options.classList.add('ignore-pref');
+    lint.removeAttribute('open');
+    lint.classList.add('ignore-pref');
+    if (!$('.usercss')) {
+      clearTimeout(scrollPointTimer);
+      scrollPointTimer = setTimeout(() => {
+        const scrollPoint = $('#header').clientHeight - 40;
+        if (window.scrollY >= scrollPoint && !$('.fixed-header') && linterEnabled) {
+          body.classList.add('fixed-header');
+        }
+      }, 250);
+      window.addEventListener('scroll', fixedHeader);
+    }
+  } else {
+    body.classList.remove('compact-layout');
+    body.classList.remove('fixed-header');
+    window.removeEventListener('scroll', fixedHeader);
+    if (shortViewportLinter && linterEnabled || shortViewportNoLinter && !linterEnabled) {
+      options.removeAttribute('open');
+      options.classList.add('ignore-pref');
+      if (prefs.get('editor.lint.expanded')) {
+        lint.setAttribute('open', '');
+      }
+    } else {
+      options.classList.remove('ignore-pref');
+      lint.classList.remove('ignore-pref');
+      if (prefs.get('editor.options.expanded')) {
+        options.setAttribute('open', '');
+      }
+      if (prefs.get('editor.lint.expanded')) {
+        lint.setAttribute('open', '');
+      }
+    }
+  }
+}
+
 function isWindowMaximized() {
   return (
     window.screenX <= 0 &&

edit/linter-report.js

@@ -37,7 +37,7 @@ Object.assign(linter, (() => {
   function updateCount() {
     const issueCount = Array.from(tables.values())
       .reduce((sum, table) => sum + table.trs.length, 0);
-    $('#lint').classList.toggle('hidden', issueCount === 0);
+    $('#lint').classList.toggle('hidden-unless-compact', issueCount === 0);
     $('#issue-count').textContent = issueCount;
   }
 
@@ -148,7 +148,7 @@ Object.assign(linter, (() => {
         col.textContent = anno.from.ch + 1;
         message.title = clipString(anno.message, 1000) +
           (anno.rule ? `\n(${anno.rule})` : '');
-        message.textContent = clipString(anno.message, 100);
+        message.textContent = clipString(anno.message, 100).replace(/ at line.*/, '');
       }
     }
   }

js/dom.js

@@ -314,7 +314,9 @@ function initCollapsibles({bindClickOn = 'h2'} = {}) {
   }
 
   function saveState(el) {
-    prefs.set(el.dataset.pref, el.open);
+    if (!el.classList.contains('ignore-pref')) {
+      prefs.set(el.dataset.pref, el.open);
+    }
   }
 }