Skip to content

Additional Highlights fail when belonging to a different scroll parent than the attachTo Element #3344

@JeremyWhiteDev

Description

@JeremyWhiteDev

Issue

When using the Additional Highlights feature, additional highlights aren't drawn if they are outside the "main" attachTo element's Scroll Parent.

Image

This is a picture of the setup. This picture shown shows the two highlights, only because I've employed a workaround, which is described below.

The workaround that worked (for this specific case) was to switch the elements that were being targeted. I could use an attachTo element that belonged to a scrollParent that contained the additionalHighlight elements, rather than targeting an element to attachTo that had a scrollParent that excluded the additionalHighlights elements. This worked when there's one scroll container causing the issue. But if there's multiple scroll containers being targets for attachTo/additionalHighlights, i'd expect this to break again.

Expected culprit

Apologies, as I'm debugging the ESM build code itself, but the issue seems to be with this implementation:

function positionModal (modalOverlayOpeningPadding = 0, modalOverlayOpeningRadius = 0, modalOverlayOpeningXOffset = 0, modalOverlayOpeningYOffset = 0, scrollParent, targetElement, extraHighlights) {
        //... abridged
        for (const element of elementsToHighlight) {
        //... abridged

                const {
                    y,
                    height
                } = _getVisibleHeight(element, scrollParent);
                const {
                    x,
                    width,
                    left
                } = element.getBoundingClientRect();

                // Check if the element is contained by another element
                const isContained = elementsToHighlight.some(otherElement => {
                    if (otherElement === element) return false;
                    const otherRect = otherElement.getBoundingClientRect();
                    return x >= otherRect.left && x + width <= otherRect.right && y >= otherRect.top && y + height <= otherRect.bottom;
                });
                if (isContained) continue;

                // getBoundingClientRect is not consistent. Some browsers use x and y, while others use left and top
                openingProperties.push({
                    width: width + modalOverlayOpeningPadding * 2,
                    height: height + modalOverlayOpeningPadding * 2,
                    x: (x || left) + modalOverlayOpeningXOffset - modalOverlayOpeningPadding,
                    y: y + modalOverlayOpeningYOffset - modalOverlayOpeningPadding,
                    r: modalOverlayOpeningRadius
                });
            }
        } else {
            closeModalOpening();
        }
    }

Scroll parent in this case is determined outside this function, and passed in. The "additional highlights" are then processed with the main "attachTo" element's scroll parent, and since they aren't "visible" based on the coords of the scroll parent, there openingProperties end up having 0 value for height.

Proposed solution

resolving the scrollParent for each highlight element, rather than using the main attachTo element's scrollParent. Would be a signature change, but resolving the scrollParent within this fn seems better than passing it in. In pseudocode:

function positionModal (modalOverlayOpeningPadding = 0, modalOverlayOpeningRadius = 0, modalOverlayOpeningXOffset = 0, modalOverlayOpeningYOffset = 0, targetElement, extraHighlights) {
        //... abridged
        for (const element of elementsToHighlight) {
        //... abridged

               const scrollParent = _getScrollParent(element); // <--- resolve scrollParent independently for each element

                const {
                    y,
                    height
                } = _getVisibleHeight(element, scrollParent);
                const {
                    x,
                    width,
                    left
                } = element.getBoundingClientRect();

                // Check if the element is contained by another element
                const isContained = elementsToHighlight.some(otherElement => {
                    if (otherElement === element) return false;
                    const otherRect = otherElement.getBoundingClientRect();
                    return x >= otherRect.left && x + width <= otherRect.right && y >= otherRect.top && y + height <= otherRect.bottom;
                });
                if (isContained) continue;

                // getBoundingClientRect is not consistent. Some browsers use x and y, while others use left and top
                openingProperties.push({
                    width: width + modalOverlayOpeningPadding * 2,
                    height: height + modalOverlayOpeningPadding * 2,
                    x: (x || left) + modalOverlayOpeningXOffset - modalOverlayOpeningPadding,
                    y: y + modalOverlayOpeningYOffset - modalOverlayOpeningPadding,
                    r: modalOverlayOpeningRadius
                });
            }
        } else {
            closeModalOpening();
        }
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions