aurelia / templating

An extensible HTML templating engine supporting databinding, custom elements, attached behaviors and more.
MIT License
116 stars 104 forks source link

pre detached event #348

Closed agentschmitt closed 8 years ago

agentschmitt commented 8 years ago

I wan't to use the ckeditor wyswig component in aurelia. Now my problem is that aurelia does only provide an detached event which is fired after my view elements are already removed from the dom. But the component i am using must be destroyed before it is removed from the dom.

Is there some other way to get an event from aurelia before an element is removed from the dom? Or could aurelia be extended with an additional lifeCycle event preDetached?

Without the ability to unregister the ckeditor component before it was removed from the dom i get errors and the component will not be correctly unregistered.

[CKEDITOR] Error code: editor-destroy-iframe. ckeditor.js?bust=22224:19 [CKEDITOR] For more information about this error go to http://docs.ckeditor.com/#!/guide/dev_errors-section-editor-destroy-iframe

<template>
    <div class="editorContainer">
        <textarea ref="element" id="${id}" value.bind="text">
        </textarea>
    </div>
</template>
/// <reference path="../../../typings/tsd.d.ts" />

import {autoinject} from "aurelia-dependency-injection";
import {bindable} from "aurelia-templating";

@autoinject
export class Editor {

    @bindable
    public text: string;
    @bindable
    public id: string = "editor";

    private element: HTMLTextAreaElement;
    private editor: CKEDITOR.editor;
    private isUpdating: boolean;

    constructor(observer: Observer) {
        this.observer = observer;
    }

    public attached(): void {
        let config: any = {};

        this.editor = CKEDITOR.replace(this.element, config);
        this.editor.on('change', this.onValueChanged.bind(this));

        this.observer.subscribe(this, "text", this.observerChanged.bind(this));
    }

    public detached(): void {
        this.observer.unSubscribe();
        this.editor.destroy(true);
        this.editor = null;
    }

    private observerChanged(): void {
        let data: string =  this.editor.getData();

        if (data !== this.text) {
            this.isUpdating = true;
            this.editor.setData(this.text, { callback: this.onValueUpdated.bind(this) });
        }
    }

    private onValueUpdated(): void {
        this.isUpdating = false;
    }

    private onValueChanged(event: CKEDITOR.eventInfo): void {
        let data: string =  event.editor.getData();

        if (!this.isUpdating && data !== this.text) {
            this.text = data;
        }
    }
}
EisenbergEffect commented 8 years ago

We don't have a hook for this because it's not available in web components and we're worried about causing integration issues. However, there is a way to hack around it if you always know that your ckeditor component will be used in a view that is either composed or routed. I'll paste here some source code for a ckeditor wrapper I wrote a couple of months back.

import {bindable, bindingMode, inject, noView, TaskQueue} from 'aurelia-framework';

CKEDITOR.config.skin = 'bootstrapck';

@inject(Element, TaskQueue)
@noView()
export class RichTextEditor {
  @bindable({ defaultBindingMode: bindingMode.twoWay }) value;

  constructor(element, taskQueue) {
    this.element = element;
    this.taskQueue = taskQueue;
    this.guard = false;
  }

  created(owningView) {
    let original = owningView.removeNodes;
    let that = this;

    owningView.removeNodes = () => {
      this.editor.destroy();
      original.call(owningView);
    };
  }

  bind() {
    this.editor = CKEDITOR.appendTo(this.element, { removePlugins: 'resize, elementspath' }, this.value);

    this.editor.on('change', () => {
      let newValue = this.editor.getData();

      if(this.value === newValue) {
        return;
      }

      this.guard = true;
      this.value = newValue;
      this.taskQueue.queueMicroTask(() => this.guard = false);
    });
  }

  valueChanged(newValue, oldValue) {
    if (this.guard || !this.editor) {
      return;
    }

    this.editor.setData(newValue);
  }
}

Notice how I use the created callback to get a reference to the view that the component is used inside of. I replace that with a custom implementation that destroys the editor before the nodes are removed. This will only work if the component is consistently used inside of routed or composed views. It won't work if the editor is used inside of a child view that isn't explicitly added/removed by the framework...at least not consistently.

We are considering how we can add more hooks in the future, particularly for this situation. It doesn't happen often but we understand the need. We have to be careful how we do it though so we don't over complicate things or cause performance issues.

glen-84 commented 5 years ago

@EisenbergEffect Any updates regarding this issue? I have a case where your solution doesn't work.

EisenbergEffect commented 5 years ago

Unfortunately, we won't be able to add the pre-detach event in the current version of Aurelia. However, it is already implemented in our vNext lifecycle (it's called detaching). @glen-84 Would you mind opening up a discussion with a few more details in your use case on aurelia.discourse.io ? The core team and community can try to work with you there to find a solution.

glen-84 commented 5 years ago

@EisenbergEffect,

Would you mind opening up a discussion ...

I'm trying to create an example on CodeSandbox, but I need to use file-loader for CKEditor, and that's not working.

I don't know if it makes sense to start a discussion when I don't even have an example to show.

FWIW, a summary:

  1. You select an item from a select input.
  2. On change, it loads a list of parameters from an API endpoint.
  3. <compose> is used to display inputs for those parameters, and some of them require CKEditor.

So, CKEditor comes and goes as you change the selection, and it should be destroyed each time. A detaching hook would be perfect.

Unfortunately, we won't be able to add the pre-detach event in the current version of Aurelia.

That's unfortunate. Is it technically impossible, or is it just that you don't want to add new "features" to vCurrent?

EisenbergEffect commented 5 years ago

@glen-84 In terms of adding the hooks to vCurrent, it's not truly impossible. However, the way that vCurrent is designed, if we were to add more lifecycle callbacks, every component in every app would suffer performance hits. At the time it was designed, it wasn't really meant to handle more hooks than what it has today. In vNext, we've re-designed how the entire component lifecycle is implemented, making it possible to get good performance while adding new hooks and capabilities during the lifecycle. Beyond that the AoT system that we're planning for vNext will be able to optimize lifecycles to remove hooks that aren't needed at runtime. So, technically possible, but very impractical in vCurrent and potentially destructive in all other scenarios besides this one.

So, for your scenario, let's see if we can come up with something. It's been a while since I've worked with CKEditor. Is this v4 or v5? Also, if you clean things up in the detached event, remind me what happens there. Does it error or something else?

glen-84 commented 5 years ago

@EisenbergEffect,

In terms of adding the hooks to vCurrent ...

Fair enough. Thanks for explaining. 🙂

It's been a while since I've worked with CKEditor. Is this v4 or v5?

v4, which can be a bit awkward to get working with webpack (which is why I needed the file-loader).

Does it error or something else?

Yes, there is an error:

[CKEDITOR] Error code: editor-destroy-iframe. [CKEDITOR] For more information about this error go to https://ckeditor.com/docs/ckeditor4/latest/guide/dev_errors.html#editor-destroy-iframe

From that URL:

The editor’s