Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nyxtom
Copy link

@nyxtom nyxtom commented Jan 20, 2021

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.

Fix template default strategy to check for buffer elements on detatch
@CLAassistant
Copy link

CLAassistant commented Jan 20, 2021

CLA assistant check
All committers have signed the CLA.

@bigopon
Copy link
Member

bigopon commented Jan 20, 2021

@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
Copy link
Author

nyxtom commented Jan 20, 2021

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;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants