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

On dialog definition event is not working #67

Closed
varunmeka opened this issue Oct 16, 2019 · 11 comments
Closed

On dialog definition event is not working #67

varunmeka opened this issue Oct 16, 2019 · 11 comments

Comments

@varunmeka
Copy link

Are you reporting a feature request or a bug?

calling on DialogDefinition event in the ckeditor4-react npm package doesn't work

Provide detailed reproduction steps (if any)

<CKEditor
data={'random'}
config={{
filebrowserUploadUrl: 'random/',
uploadUrl: '',
}}
onChange={e => {e.editor.getData (); }}
onDialogDefinition={ev => console.log (ev) }
onFileUploadRequest={e => console.log (e)}
/>

Expected result

on dialogDefinition event should be logged

Actual result

no event is seen

Other details

  • Browser: …
  • OS: …
  • Integration version: …
  • CKEditor version: …
  • Installed CKEditor plugins: …
@ishindanil
Copy link

I think your problem is related to this.
https://stackoverflow.com/a/31427109

ckeditor4-react applies handlers to an editor instance as you can see here:

this.editor.on( evtName, this.props[ propName ] );

@varunmeka
Copy link
Author

Actually I need to add some html to image plugin and change some names, I done it in onDialogDefinition Event. Is there any other way to do it

@Comandeer
Copy link
Member

Sorry for the late response!

You can use dialogDefinition event inside onBeforeLoad:

<CKEditor
	onBeforeLoad = { ( CKEDITOR ) => CKEDITOR.on( 'dialogDefinition', someFn ) }
/>

@varunmeka
Copy link
Author

I was using the same but, I am adding a custom button in image plugin some times the button is showing more than one time . I can't reproduce why this is happening since it is occuring randomly

@f1ames
Copy link
Contributor

f1ames commented Nov 12, 2019

@codeforsure that shouldn't happen TBH. Make sure that your dialogDefinition listener is attached only once. Alternatively, you can add some extra check to see if your custom button was already added in your listener.

@varunmeka
Copy link
Author

Actually, I am using custom check and it is working. But, I am not sure why the dialogDefinition is attaching 'n' times( I am loading the editor component not the whole page on event) for 'n' loads of editor component

@Comandeer
Copy link
Member

It's caused by the fact that for now onBeforeLoad runs for every editor's instance. Probably the easiest workaround would be to use some kind of a flag:

let attached = false;

<CKEditor
	onBeforeLoad = { ( CKEDITOR ) => { !attached && CKEDITOR.on( 'dialogDefinition', someFn ); attached = true; } }
/>

@f1ames
Copy link
Contributor

f1ames commented Dec 16, 2019

I assume that onBeforeLoad works like that by design, However, we lack any documentation for this behavior so it should be added describing how it works.

@f1ames
Copy link
Contributor

f1ames commented Jan 9, 2020

To sum up, the onBeforeLoad event is fired once for every editor instance it is attached to. This is the correct behavior of onBeforeLoad event.

Dialog definition is fired on global CKEDITOR namespace (so doesn't depend on specific editor instance) the first time the dialog gets opened/shown (per editor instance). Also the correct, intended behaviour.

This means that when having more than one editor instance, attaching any callback to dialogDefinition inside onBeforeLoad will attach it x times (x is the amount of editors initialized).

The first solution is to provide check if the callback was already attached as proposed in #67 (comment). Another solution could be attaching the callback to only one editor instance.

Both are fine as long as you want to apply the same dialog modifications to each editor instance. If one tries to customize differently each editor instance it will fail. So the most generic solution I'm able to think of should check if it's operating on editor instance it was attached to:

<CKEditor
    onBeforeLoad = { ( CKEDITOR, getEditorInstance ) => {
        CKEDITOR.on( 'dialogDefinition', evt => {
            if ( evt.editor === getEditorInstance() ) {
                someFn();
            }
        } ); 
    } }
/>

However, at the moment there is no possibility to check editor instance like the above. Extracted to #77.

@f1ames
Copy link
Contributor

f1ames commented Jan 9, 2020

Closing as we will proceed further in #77.

@f1ames f1ames closed this as completed Jan 9, 2020
@Darkilen
Copy link

Another solution is to use CKEDITOR.once() instead of CKEDITOR.on(), so it would run only once for this editor instance. If we have two instance with the same event (or differents events), they wouldn't override/overlaps each others.

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

No branches or pull requests

5 participants