From 92694807ea2e16102181b38d53cc62f54421a988 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 9 Oct 2024 11:44:33 +0100 Subject: [PATCH] Closes #6959 UI issue after clear cache and apply LRC with certain template (#30) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * :sparkles: fixed codacy issues --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> --- src/BeaconLrc.js | 57 ++++++++++++++++++++++ test/BeaconLrc.test.js | 104 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 157 insertions(+), 4 deletions(-) diff --git a/src/BeaconLrc.js b/src/BeaconLrc.js index 52fd10d..404297d 100644 --- 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 index d0724be..ef1f6d3 100644 --- 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(); + }) }); \ No newline at end of file