Closes #6959 UI issue after clear cache and apply LRC with certain te…

…mplate (#30) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * ✨ fixed codacy issues --------- Co-authored-by: WordPress Fan <[email protected]>
wp-media · Oct 9, 2024 · 9269480 · 9269480
1 parent 78ca211
commit 9269480
Show file tree

Hide file tree

Showing 2 changed files with 157 additions and 4 deletions.
diff --git a/src/BeaconLrc.js b/src/BeaconLrc.js
@@ -72,6 +72,57 @@ class BeaconLrc {
         return false;
     }
 
+    _checkLcrConflict(element) {
+        const conflictingElements = [];
+        const computedStyle = window.getComputedStyle(element);
+
+        const validMargins = ['marginTop', 'marginRight', 'marginBottom', 'marginLeft'];
+
+        const negativeMargins = validMargins
+            .some(margin => parseFloat(computedStyle[margin]) < 0);
+
+        const currentElementConflicts = negativeMargins ||
+          computedStyle.contentVisibility === 'auto' ||
+            computedStyle.contentVisibility === 'hidden';
+
+        if (currentElementConflicts) {
+            conflictingElements.push({
+                element,
+                conflicts: [
+                    negativeMargins && 'negative margin',
+                    computedStyle.contentVisibility === 'auto' && 'content-visibility:auto',
+                    computedStyle.contentVisibility === 'hidden' && 'content-visibility:hidden'
+                ].filter(Boolean)
+            });
+        }
+
+        Array.from(element.children).forEach(child => {
+            const childStyle = window.getComputedStyle(child);
+
+            const validMargins = ['marginTop', 'marginRight', 'marginBottom', 'marginLeft'];
+
+            const childNegativeMargins = validMargins
+                .some(margin => parseFloat(childStyle[margin]) < 0);
+
+            const childConflicts = childNegativeMargins ||
+              childStyle.position === 'absolute' ||
+                childStyle.position === 'fixed';
+
+            if (childConflicts) {
+                conflictingElements.push({
+                    element: child,
+                    conflicts: [
+                        childNegativeMargins && 'negative margin',
+                        childStyle.position === 'absolute' && 'position:absolute',
+                        childStyle.position === 'fixed' && 'position:fixed'
+                    ].filter(Boolean)
+                });
+            }
+        });
+
+        return conflictingElements;
+    }
+
     _processElements(elements) {
         elements.forEach(({ element, depth, distance, hash }) => {
             if (this._shouldSkipElement(element, this.config.exclusions || [])) {
@@ -82,6 +133,12 @@ class BeaconLrc {
                 return;
             }
 
+            const conflicts = this._checkLcrConflict(element);
+            if (conflicts.length > 0) {
+                this.logger.logMessage('Skipping element due to conflicts:', conflicts);
+                return;
+            }
+
             const can_push_hash = element.parentElement && this._getElementDistance(element.parentElement) < this.config.lrc_threshold && distance >= this.config.lrc_threshold;
 
             const color = can_push_hash ? "green" : distance === 0 ? "red" : "";

diff --git a/test/BeaconLrc.test.js b/test/BeaconLrc.test.js
@@ -16,7 +16,8 @@ describe('BeaconLrc', function() {
                 },
                 getAttribute: () => 'hash1',
                 hasAttribute: () => true,
-                dataset: { rocketLocationHash: 'hash1' }
+                dataset: { rocketLocationHash: 'hash1' },
+                children: []
             },
             {
                 getBoundingClientRect: () => {
@@ -26,7 +27,8 @@ describe('BeaconLrc', function() {
                 },
                 getAttribute: () => 'hash2',
                 hasAttribute: () => true,
-                dataset: { rocketLocationHash: 'hash2' }
+                dataset: { rocketLocationHash: 'hash2' },
+                children: []
             },
             {
                 getBoundingClientRect: () => {
@@ -36,7 +38,8 @@ describe('BeaconLrc', function() {
                 },
                 getAttribute: () => 'hash3',
                 hasAttribute: () => true,
-                dataset: { rocketLocationHash: 'hash3' }
+                dataset: { rocketLocationHash: 'hash3' },
+                children: []
             },
             {
                 getBoundingClientRect: () => {
@@ -46,7 +49,8 @@ describe('BeaconLrc', function() {
                 },
                 getAttribute: () => 'hash4',
                 hasAttribute: () => true,
-                dataset: { rocketLocationHash: 'hash4' }
+                dataset: { rocketLocationHash: 'hash4' },
+                children: []
             },
         ];
 
@@ -74,6 +78,18 @@ describe('BeaconLrc', function() {
 
         // Mocking window.pageYOffset
         global.window = { pageYOffset: 100, innerHeight: 500 };
+
+        if (typeof global.window.getComputedStyle !== 'function') {
+            global.window.getComputedStyle = () => ({
+                marginTop: '0px',
+                marginRight: '0px',
+                marginBottom: '0px',
+                marginLeft: '0px',
+                contentVisibility: 'visible',
+                position: 'static',
+                overflow: 'visible'
+            });
+        }
     });
 
     afterEach(function() {
@@ -232,4 +248,84 @@ describe('BeaconLrc', function() {
 
         _getElementXPathStub.restore();
     });
+
+    it('should detect conflict when element has negative margins', () => {
+        const element = mockElements[0];
+
+        sinon.stub(window, 'getComputedStyle').callsFake(() => ({
+            marginTop: '-10px',
+            marginRight: '0px',
+            marginBottom: '0px',
+            marginLeft: '0px',
+            contentVisibility: 'visible',
+            position: 'static',
+            overflow: 'visible'
+        }));
+
+        const result = beaconLrc._checkLcrConflict(element);
+
+        assert.strictEqual(result.length, 1);
+        assert.strictEqual(result[0].conflicts.includes('negative margin'), true);
+        window.getComputedStyle.restore();
+    });
+
+    it('should detect conflict when content visibility is hidden', () => {
+        const element = mockElements[0];
+
+        sinon.stub(window, 'getComputedStyle').callsFake(() => ({
+            marginTop: '0px',
+            marginRight: '0px',
+            marginBottom: '0px',
+            marginLeft: '0px',
+            contentVisibility: 'hidden',
+            position: 'static',
+            overflow: 'visible'
+        }));
+
+        const result = beaconLrc._checkLcrConflict(element);
+
+        assert.strictEqual(result.length, 1);
+        assert.strictEqual(result[0].conflicts.includes('content-visibility:hidden'), true);
+        window.getComputedStyle.restore();
+    });
+
+    it('should detect conflict when child has negative margins', () => {
+        const element = mockElements[0];
+
+        const child = {
+            getBoundingClientRect: () => ({ top: 500 }),
+            getAttribute: () => null,
+            hasAttribute: () => false,
+            children: []
+        };
+        element.children = [child];
+
+        sinon.stub(window, 'getComputedStyle')
+            .onCall(0).returns({
+            marginTop: '0px',
+            marginRight: '0px',
+            marginBottom: '0px',
+            marginLeft: '0px',
+            contentVisibility: 'visible',
+            position: 'static',
+            overflow: 'visible'
+        })
+            .onCall(1).returns({
+            marginTop: '-20px',
+            marginRight: '0px',
+            marginBottom: '0px',
+            marginLeft: '0px',
+            contentVisibility: 'visible',
+            position: 'absolute',
+            overflow: 'visible'
+        });
+
+        const result = beaconLrc._checkLcrConflict(element);
+
+        assert.strictEqual(result.length, 1);
+        assert.strictEqual(result[0].element, child);
+        assert.strictEqual(result[0].conflicts.includes('negative margin'), true);
+
+        window.getComputedStyle.restore();
+    })
 });