fix: consider decimal values of scrollLeft and scrollTop (#5732) (#5743)

vaadin · Apr 4, 2023 · ec2c935 · ec2c935
1 parent aabe6a4
commit ec2c935
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 10 deletions.
diff --git a/packages/tabs/src/vaadin-tabs.js b/packages/tabs/src/vaadin-tabs.js
@@ -249,8 +249,17 @@ class Tabs extends ResizeMixin(ElementMixin(ListMixin(ThemableMixin(PolymerEleme
       : this.__getNormalizedScrollLeft(this._scrollerElement);
     const scrollSize = this._vertical ? this._scrollerElement.scrollHeight : this._scrollerElement.scrollWidth;
 
-    let overflow = scrollPosition > 0 ? 'start' : '';
-    overflow += scrollPosition + this._scrollOffset < scrollSize ? ' end' : '';
+    // Note that we are not comparing floored scrollPosition to be greater than zero here, which would make a natural
+    // sense, but to be greater than one intentionally. There is a known bug in Chromium browsers on Linux/Mac
+    // (https://bugs.chromium.org/p/chromium/issues/detail?id=1123301), which returns invalid value of scrollLeft when
+    // text direction is RTL. The value is off by one pixel in that case. Comparing scrollPosition to be greater than
+    // one on the following line is a workaround for that bug. Comparing scrollPosition to be greater than one means,
+    // that the left overflow and left arrow will be displayed "one pixel" later than normal. In other words, if the tab
+    // scroller element is scrolled just by 1px, the overflow is not yet showing.
+    let overflow = Math.floor(scrollPosition) > 1 ? 'start' : '';
+    if (Math.ceil(scrollPosition) < Math.ceil(scrollSize - this._scrollOffset)) {
+      overflow += ' end';
+    }
 
     if (this.__direction === 1) {
       overflow = overflow.replace(/start|end/gi, (matched) => {

diff --git a/packages/tabs/test/tabs.test.js b/packages/tabs/test/tabs.test.js
@@ -31,6 +31,7 @@ describe('tabs', () => {
       <vaadin-tabs>
         <vaadin-tab>Foo</vaadin-tab>
         <vaadin-tab>Bar</vaadin-tab>
+        <vaadin-tab>Some</vaadin-tab>
         <span></span>
         <vaadin-tab disabled>Baz</vaadin-tab>
         <vaadin-tab>
@@ -59,7 +60,7 @@ describe('tabs', () => {
 
   describe('items', () => {
     it('should only add vaadin-tab components to items', () => {
-      expect(tabs.items.length).to.equal(4);
+      expect(tabs.items.length).to.equal(5);
       tabs.items.forEach((item) => {
         expect(item.tagName.toLowerCase()).to.equal('vaadin-tab');
       });
@@ -114,6 +115,10 @@ describe('tabs', () => {
             await nextFrame();
           });
 
+          afterEach(() => {
+            document.body.style.zoom = '';
+          });
+
           it(`when orientation=${orientation} should have overflow="end" if scroll is at the beginning`, () => {
             expect(tabs.getAttribute('overflow')).to.be.equal('end');
           });
@@ -127,18 +132,24 @@ describe('tabs', () => {
             tabs._scroll(horizontalRtl ? -2 : 2);
           });
 
-          // TODO: passes locally but fails in GitHub Actions due to 1px difference.
-          const chrome = /HeadlessChrome/.test(navigator.userAgent);
-          (horizontalRtl && chrome ? it.skip : it)(
-            `when orientation=${orientation} should have overflow="start" if scroll is at the end`,
-            (done) => {
+          it(`when orientation=${orientation} should have overflow="start" if scroll is at the end`, (done) => {
+            listenOnce(tabs._scrollerElement, 'scroll', () => {
+              expect(tabs.getAttribute('overflow')).to.be.equal('start');
+              done();
+            });
+            tabs._scroll(horizontalRtl ? -200 : 200);
+          });
+
+          [1.25, 1.33, 1.5, 1.75].forEach((zoomLevel) => {
+            it(`when orientation=${orientation} should have overflow="start" if scroll is at the end on page zoomed to ${zoomLevel}`, (done) => {
+              document.body.style.zoom = zoomLevel;
               listenOnce(tabs._scrollerElement, 'scroll', () => {
                 expect(tabs.getAttribute('overflow')).to.be.equal('start');
                 done();
               });
               tabs._scroll(horizontalRtl ? -200 : 200);
-            },
-          );
+            });
+          });
 
           it(`when orientation=${orientation} should not have overflow="start" when over-scrolling`, () => {
             const scroll = tabs._scrollerElement;