aurelia / ui-virtualization

A plugin that provides a virtualized repeater and other virtualization services.
MIT License
90 stars 45 forks source link

Fix templateStrategy removeBuffers to check whether parent contains elements before removing #201

Open nyxtom opened 3 years ago

nyxtom commented 3 years ago

It's possible to run into an error where detaching the virtual repeat will lose context of the bottom and top buffer elements (due to underlying conditional changes possibly). As a result, the templateStrategy.removeBuffers will throw an error when you attempt to call removeChild when the element is not a child of the given parent element.

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

bigopon commented 3 years ago

@nyxtom thanks for this PR. This probably only happens when there's some html mutation outside of Aurelia context, and results in the elements being removed without signalling the virtual repeater. Maybe we can do this, I'm a bit hesitant to add this defensive code though, can you describe the issues you are seeing a bit more?

nyxtom commented 3 years ago

I’m currently using it within a tree view control which has nested virtual repeats according their children. It’s a bit of an odd situation but it involves using a custom template in a slot and binding it in through the aurelia processContent behavior. I can’t seem to code around it without adding this fail safe at the moment unfortunately

tree-view.html

<template class="tree-view ${disabled ? 'disabled' : ''}">
    <slot></slot>
    <div if.bind="!isLoading">
        <div virtual-repeat.for="node of state.nodes">
            <tree-view-node model.bind="node" api.bind="api"></tree-view-node>
        </div>
    </div>
</template>

tree-view-node.html

<template class="${model.hasChildren ? 'tree-view-node--node' : 'tree-view-node--leaf'} ${(!model.isSelected && model.hasSelectedChildren) ? 'tree-view-node--selected-partial' : ''} ${model.isSelected ? 'tree-view-node--selected': ''} ${model.isFocused ? 'tree-view-node--focused': ''} ${model.isExpanded ? 'tree-view-node--expanded' : ''}">
    <template if.bind="!hasTreeViewTemplate" containerless>
        <div show.bind="model.isVisible && (model.isMatch || !settings.filtering)" class="tree-view-node">
            <div if.bind="!hasTreeViewNodeTemplate" class="tree-view-node-title-wrapper">
                <span if.bind="model.hasChildren"
                        click.delegate="toggleExpanded()"
                        class="tree-view-node-arrow ${model.isExpanded ? 'tree-view-node-arrow--expanded' : '' } fa ${model.isLoading ? 'fa-refresh' : 'fa-angle-right'}">
                </span>
                <span click.delegate="focus()" class="tree-view-node-title" title="${model.payload.title}">
                    <label if.bind="settings.multiSelect">
                        <input type="checkbox" checked.bind="model.isSelected" />
                    </label>
                    <span class="tree-view-node-title-text">
                        ${model.payload.title}
                    </span>
                </span>
            </div>
            <div else ref="nodeTemplateTarget" class="tree-view-node-title-wrapper"></div>

            <div if.bind="model.hasChildren"
                class="tree-view-node-children">
                <div virtual-repeat.for="node of model.visibleChildren">
                    <tree-view-node model.bind="node" api.bind="api"></tree-view-node>
                </div>
            </div>
        </div>
    </template>
    <div else ref="templateTarget" containerless></div>
</template>

tree-view-node.js

    attached() {
        if (this.viewSlot) {
            this.viewSlot.detached();
            this.viewSlot.unbind();
            this.viewSlot.removeAll();
        }
        if (this.hasTreeViewTemplate) {
            let templateInfo = this.settings.templateInfo;
            this.attachTemplate(templateInfo, this.templateTarget);
        }
        if (this.hasTreeViewNodeTemplate) {
            let templateInfo = this.settings.nodeTemplateInfo;
            this.attachTemplate(templateInfo, this.nodeTemplateTarget);
        }
    }

    attachTemplate(templateInfo, templateTarget) {
        let viewFactory = this.viewCompiler.compile(`<template>${templateInfo.template}</template>`, this.viewResources);
        let view = viewFactory.create(this.container);
        this.viewSlot = new ViewSlot(templateTarget, true);
        this.viewSlot.add(view);
        this.viewSlot.bind(this, createOverrideContext(this, createOverrideContext(templateInfo.viewModel)));
        this.viewSlot.attached();
    }

tree-view-node-template.js

import { inject, bindable, processContent, noView, customElement, TargetInstruction, Parent } from 'aurelia-framework';
import { TreeView } from './tree-view';

@customElement('tree-view-node-template')
@noView()
@processContent((compiler, resources, element, instruction) => {
    let html = element.innerHTML;
    if (html !== '') {
        instruction.template = html;
    }
    element.innerHTML = '';
})
@inject(TargetInstruction, Parent.of(TreeView))
export class TreeViewNodeTemplate {
    @bindable template;
    @bindable model;

    constructor(targetInstruction, treeView) {
        this.treeView = treeView;
        this.template = targetInstruction.elementInstruction.template;
    }
}