Honor macOS "Show scrollbars only when scrolling" setting

Previously, we were hiding scrollbars when their height/width was 0. On
macOS, however, users can decide to only show scrollbars while
scrolling, which causes Atom to detect scrollbars as being invisible
during measurements. As a result, we were mistakenly setting the
visibility property to `hidden` when this setting was on, thus
preventing users from seeing the scrollbar on scroll.

With this commit we are changing the dummy scrollbar components to only
become invisible when the content is not scrollable rather than when the
scrollbars have zero width or height.

As part of this, we have also renamed the
`is{Horizontal,Vertical}ScrollbarVisible` functions to
`canScroll{Horizontally,Vertically}`, to better express their intent.

spec/text-editor-component-spec.js

@@ -688,8 +688,8 @@ describe('TextEditorComponent', () => {
       await component.getNextUpdatePromise()
       element.style.width = component.getGutterContainerWidth() + component.getContentHeight() - 20 + 'px'
       await component.getNextUpdatePromise()
-      expect(component.isHorizontalScrollbarVisible()).toBe(true)
-      expect(component.isVerticalScrollbarVisible()).toBe(false)
+      expect(component.canScrollHorizontally()).toBe(true)
+      expect(component.canScrollVertically()).toBe(false)
       expect(element.offsetHeight).toBe(component.getContentHeight() + component.getHorizontalScrollbarHeight() + 2 * editorPadding)
 
       // When a vertical scrollbar is visible, autoWidth accounts for it
@@ -697,8 +697,8 @@ describe('TextEditorComponent', () => {
       await component.getNextUpdatePromise()
       element.style.height = component.getContentHeight() - 20
       await component.getNextUpdatePromise()
-      expect(component.isHorizontalScrollbarVisible()).toBe(false)
-      expect(component.isVerticalScrollbarVisible()).toBe(true)
+      expect(component.canScrollHorizontally()).toBe(false)
+      expect(component.canScrollVertically()).toBe(true)
       expect(element.offsetWidth).toBe(
         component.getGutterContainerWidth() +
         component.getContentWidth() +
@@ -898,8 +898,8 @@ describe('TextEditorComponent', () => {
       editor.setText('x'.repeat(20) + 'y'.repeat(20))
       await component.getNextUpdatePromise()
 
-      expect(component.isHorizontalScrollbarVisible()).toBe(false)
-      expect(component.isVerticalScrollbarVisible()).toBe(false)
+      expect(component.canScrollVertically()).toBe(false)
+      expect(component.canScrollHorizontally()).toBe(false)
       expect(component.refs.horizontalScrollbar).toBeUndefined()
       expect(component.refs.verticalScrollbar).toBeUndefined()
     })

src/text-editor-component.js

@@ -381,7 +381,10 @@ class TextEditorComponent {
       this.measureGutterDimensions()
       this.remeasureGutterDimensions = false
     }
-    const wasHorizontalScrollbarVisible = this.isHorizontalScrollbarVisible()
+    const wasHorizontalScrollbarVisible = (
+      this.canScrollHorizontally() &&
+      this.getHorizontalScrollbarHeight() > 0
+    )
 
     this.measureLongestLineWidth()
     this.measureHorizontalPositions()
@@ -391,7 +394,12 @@ class TextEditorComponent {
       this.derivedDimensionsCache = {}
       const {screenRange, options} = this.pendingAutoscroll
       this.autoscrollHorizontally(screenRange, options)
-      if (!wasHorizontalScrollbarVisible && this.isHorizontalScrollbarVisible()) {
+
+      const isHorizontalScrollbarVisible = (
+        this.canScrollHorizontally() &&
+        this.getHorizontalScrollbarHeight() > 0
+      )
+      if (!wasHorizontalScrollbarVisible && isHorizontalScrollbarVisible) {
         this.autoscrollVertically(screenRange, options)
       }
       this.pendingAutoscroll = null
@@ -439,13 +447,13 @@ class TextEditorComponent {
     if (this.hasInitialMeasurements) {
       if (model.getAutoHeight()) {
         clientContainerHeight = this.getContentHeight()
-        if (this.isHorizontalScrollbarVisible()) clientContainerHeight += this.getHorizontalScrollbarHeight()
+        if (this.canScrollHorizontally()) clientContainerHeight += this.getHorizontalScrollbarHeight()
         clientContainerHeight += 'px'
       }
       if (model.getAutoWidth()) {
         style.width = 'min-content'
         clientContainerWidth = this.getGutterContainerWidth() + this.getContentWidth()
-        if (this.isVerticalScrollbarVisible()) clientContainerWidth += this.getVerticalScrollbarWidth()
+        if (this.canScrollVertically()) clientContainerWidth += this.getVerticalScrollbarWidth()
         clientContainerWidth += 'px'
       } else {
         style.width = this.element.style.width
@@ -716,18 +724,21 @@ class TextEditorComponent {
     if (this.shouldRenderDummyScrollbars && !this.props.model.isMini()) {
       let scrollHeight, scrollTop, horizontalScrollbarHeight
       let scrollWidth, scrollLeft, verticalScrollbarWidth, forceScrollbarVisible
+      let canScrollHorizontally, canScrollVertically
 
       if (this.hasInitialMeasurements) {
         scrollHeight = this.getScrollHeight()
         scrollWidth = this.getScrollWidth()
         scrollTop = this.getScrollTop()
         scrollLeft = this.getScrollLeft()
+        canScrollHorizontally = this.canScrollHorizontally()
+        canScrollVertically = this.canScrollVertically()
         horizontalScrollbarHeight =
-          this.isHorizontalScrollbarVisible()
+          canScrollHorizontally
           ? this.getHorizontalScrollbarHeight()
           : 0
         verticalScrollbarWidth =
-          this.isVerticalScrollbarVisible()
+          canScrollVertically
           ? this.getVerticalScrollbarWidth()
           : 0
         forceScrollbarVisible = this.remeasureScrollbars
@@ -741,10 +752,10 @@ class TextEditorComponent {
           orientation: 'vertical',
           didScroll: this.didScrollDummyScrollbar,
           didMouseDown: this.didMouseDownOnContent,
+          canScroll: canScrollVertically,
           scrollHeight,
           scrollTop,
           horizontalScrollbarHeight,
-          verticalScrollbarWidth,
           forceScrollbarVisible
         }),
         $(DummyScrollbarComponent, {
@@ -752,9 +763,9 @@ class TextEditorComponent {
           orientation: 'horizontal',
           didScroll: this.didScrollDummyScrollbar,
           didMouseDown: this.didMouseDownOnContent,
+          canScroll: canScrollHorizontally,
           scrollWidth,
           scrollLeft,
-          horizontalScrollbarHeight,
           verticalScrollbarWidth,
           forceScrollbarVisible
         })
@@ -2568,7 +2579,7 @@ class TextEditorComponent {
   }
 
   getScrollContainerClientWidth () {
-    if (this.isVerticalScrollbarVisible()) {
+    if (this.canScrollVertically()) {
       return this.getScrollContainerWidth() - this.getVerticalScrollbarWidth()
     } else {
       return this.getScrollContainerWidth()
@@ -2576,14 +2587,14 @@ class TextEditorComponent {
   }
 
   getScrollContainerClientHeight () {
-    if (this.isHorizontalScrollbarVisible()) {
+    if (this.canScrollHorizontally()) {
       return this.getScrollContainerHeight() - this.getHorizontalScrollbarHeight()
     } else {
       return this.getScrollContainerHeight()
     }
   }
 
-  isVerticalScrollbarVisible () {
+  canScrollVertically () {
     const {model} = this.props
     if (model.isMini()) return false
     if (model.getAutoHeight()) return false
@@ -2594,7 +2605,7 @@ class TextEditorComponent {
     )
   }
 
-  isHorizontalScrollbarVisible () {
+  canScrollHorizontally () {
     const {model} = this.props
     if (model.isMini()) return false
     if (model.getAutoWidth()) return false
@@ -2943,8 +2954,8 @@ class DummyScrollbarComponent {
   render () {
     const {
       orientation, scrollWidth, scrollHeight,
-      verticalScrollbarWidth, horizontalScrollbarHeight, forceScrollbarVisible,
-      didScroll, didMouseDown
+      verticalScrollbarWidth, horizontalScrollbarHeight,
+      canScroll, forceScrollbarVisible, didScroll, didMouseDown
     } = this.props
 
     const outerStyle = {
@@ -2953,6 +2964,8 @@ class DummyScrollbarComponent {
       zIndex: 1,
       willChange: 'transform'
     }
+    if (!canScroll) outerStyle.visibility = 'hidden'
+
     const innerStyle = {}
     if (orientation === 'horizontal') {
       let right = (verticalScrollbarWidth || 0)
@@ -2963,9 +2976,6 @@ class DummyScrollbarComponent {
       outerStyle.overflowY = 'hidden'
       outerStyle.overflowX = forceScrollbarVisible ? 'scroll' : 'auto'
       outerStyle.cursor = 'default'
-      if (horizontalScrollbarHeight === 0) {
-        outerStyle.visibility = 'hidden'
-      }
       innerStyle.height = '15px'
       innerStyle.width = (scrollWidth || 0) + 'px'
     } else {
@@ -2977,9 +2987,6 @@ class DummyScrollbarComponent {
       outerStyle.overflowX = 'hidden'
       outerStyle.overflowY = forceScrollbarVisible ? 'scroll' : 'auto'
       outerStyle.cursor = 'default'
-      if (verticalScrollbarWidth === 0) {
-        outerStyle.visibility = 'hidden'
-      }
       innerStyle.width = '15px'
       innerStyle.height = (scrollHeight || 0) + 'px'
     }