Skip to content

Commit

Permalink
Spacefinder handle opponents/candidates overlapping (#1377)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jakeii authored May 14, 2024
1 parent b8d0840 commit 6758606
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/itchy-cows-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@guardian/commercial': minor
---

Spacefinder - Handle opponents/candidates overlapping
3 changes: 2 additions & 1 deletion playwright/tests/front-inline-slots.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ test.describe('Slots and iframes load on fronts pages', () => {
});
});

test.describe('Slots and iframes load on tag pages', () => {
// currently no banners on tag fronts
test.skip('Slots and iframes load on tag pages', () => {
tagPages.forEach(({ path }) => {
test(`fronts-banner ads are loaded on ${path}`, async ({ page }) => {
await loadPage(page, path);
Expand Down
2 changes: 1 addition & 1 deletion src/insert/spacefinder/article.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const addDesktopInline1 = (fillSlot: FillAdSlot): Promise<boolean> => {
minBelowSlot: 600,
},
'figure.element--supporting': {
minAboveSlot: 500,
minAboveSlot: 100,
minBelowSlot: 0,
},
},
Expand Down
21 changes: 16 additions & 5 deletions src/insert/spacefinder/spacefinder-debug-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ const exclusionTypes = {
colour: colours.blue,
reason: 'Too close to other element',
},
overlaps: {
colour: colours.blue,
reason: 'Overlaps other element',
},
} as const;

const isExclusionType = (type: string): type is keyof typeof exclusionTypes =>
Expand All @@ -72,7 +76,9 @@ const addOverlay = (element: HTMLElement, text: string) => {

const addHoverListener = (
candidate: HTMLElement,
tooClose: Exclude<SpacefinderItem['meta'], undefined>['tooClose'],
tooClose: Exclude<SpacefinderItem['meta'], undefined>[
| 'tooClose'
| 'overlaps'],
pass: SpacefinderPass,
) => {
tooClose.forEach((opponent) => {
Expand Down Expand Up @@ -100,10 +106,12 @@ const addHoverListener = (
}

opponent.element.classList.add('blocking-element');
addOverlay(
opponent.element,
`${opponent.actual}px/${opponent.required}px`,
);
if (opponent.actual && opponent.required) {
addOverlay(
opponent.element,
`${opponent.actual}px/${opponent.required}px`,
);
}
});

candidate.addEventListener('mouseleave', () => {
Expand Down Expand Up @@ -138,6 +146,9 @@ const annotateExclusions = (
element.setAttribute(`data-sfdebug-${pass}`, 'isStartAt');
} else if (type) {
element.setAttribute(`data-sfdebug-${pass}`, key);
} else if (meta && meta.overlaps.length > 0) {
element.setAttribute(`data-sfdebug-${pass}`, 'overlaps');
addHoverListener(element, meta.overlaps, pass);
} else if (meta && meta.tooClose.length > 0) {
element.setAttribute(`data-sfdebug-${pass}`, 'tooClose');
addHoverListener(element, meta.tooClose, pass);
Expand Down
75 changes: 48 additions & 27 deletions src/insert/spacefinder/spacefinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,19 @@ type RuleSpacing = {
bypassMinBelow?: string;
};

type SpacefinderMetaItem = {
required?: number;
actual?: number;
element: HTMLElement;
};

type SpacefinderItem = {
top: number;
bottom: number;
element: HTMLElement;
meta?: {
tooClose: Array<{
required: number;
actual: number;
element: HTMLElement;
}>;
meta: {
tooClose: SpacefinderMetaItem[];
overlaps: SpacefinderMetaItem[];
};
};

Expand Down Expand Up @@ -313,29 +316,45 @@ const testCandidate = (
return true;
}

const isOpponentBelow = opponent.bottom > candidate.bottom;

const pass = isTopOfCandidateFarEnoughFromOpponent(
candidate,
opponent,
rule,
isOpponentBelow,
);
const isOpponentBelow =
opponent.bottom > candidate.bottom && opponent.top > candidate.bottom;
const isOpponentAbove =
opponent.top < candidate.top && opponent.bottom < candidate.top;

// this can happen when the an opponent like an image or interactive is floated right
const opponentOverlaps =
(isOpponentAbove && isOpponentBelow) ||
(!isOpponentAbove && !isOpponentBelow);

const pass =
!opponentOverlaps &&
isTopOfCandidateFarEnoughFromOpponent(
candidate,
opponent,
rule,
isOpponentBelow,
);

if (!pass) {
// if the test fails, add debug information to the candidate metadata
const required = isOpponentBelow
? rule.minBelowSlot
: rule.minAboveSlot;
const actual = isOpponentBelow
? opponent.top - candidate.top
: candidate.top - opponent.bottom;

candidate.meta?.tooClose.push({
required,
actual,
element: opponent.element,
});
if (opponentOverlaps) {
candidate.meta.overlaps.push({
element: opponent.element,
});
} else {
// if the test fails, add debug information to the candidate metadata
const required = isOpponentBelow
? rule.minBelowSlot
: rule.minAboveSlot;
const actual = isOpponentBelow
? opponent.top - candidate.top
: candidate.top - opponent.bottom;

candidate.meta.tooClose.push({
required,
actual,
element: opponent.element,
});
}
}

return pass;
Expand Down Expand Up @@ -502,6 +521,7 @@ const getDimensions = (element: HTMLElement): Readonly<SpacefinderItem> =>
element,
meta: {
tooClose: [],
overlaps: [],
},
});

Expand Down Expand Up @@ -609,4 +629,5 @@ export type {
SpacefinderItem,
SpacefinderExclusions,
SpacefinderPass,
SpacefinderMetaItem,
};

0 comments on commit 6758606

Please sign in to comment.