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

Support setting data attributes on image from upload adapters #5204

Closed
darylteo opened this issue Sep 4, 2019 · 21 comments · Fixed by #9390
Closed

Support setting data attributes on image from upload adapters #5204

darylteo opened this issue Sep 4, 2019 · 21 comments · Fixed by #9390
Assignees
Labels
package:image type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@darylteo
Copy link

darylteo commented Sep 4, 2019

Current upload() return

        return {
            default: 'image src',
            360: '...',
            600: '...', // etc.
        };

We currently have a situation where I would like to save the original path of the uploaded image, but the images have to be served through a CDN.

It would be nice if I could save the original path as a data attribute. I've had a look at https://github.com/ckeditor/ckeditor5-image/blob/8fa4acf7850e958526d0f049899f88cb91282a74/src/imageupload/imageuploadediting.js to find the relevant code and it comes down to the _parseAndSetSrcsetAttributeOnImage function which sets the srcset attributes for numeric keys.

If we could have support another attributes property where this information could be passed through, allowing the Custom Upload Adapter to supply any number of attributes to save, that would be very handy. Could be used to set the sizes attribute, for example, which this plugin currently does not do.

Proposed upload() return

        return {
            default: 'image src',
            360: '...',
            600: '...', // etc.

            attributes: { 
               'data-original-path': 'image path',
            },
        };

My current work around is to use downcast/upcast to parse the src and reverse engineer the path that way, which is slightly annoying but not impossible, I suppose. Just feels easier to get it from the horse's mouth.

Thank you very much!

@Mgsy
Copy link
Member

Mgsy commented Sep 4, 2019

cc @oleq

@oleq
Copy link
Member

oleq commented Sep 20, 2019

Can you share some code? Because I'm at a loss for what you use, what you want to do and what is the actual issue here.

@darylteo
Copy link
Author

darylteo commented Sep 23, 2019

Here's the workaround we are now using in production, which we derived from the ImageUpload plugin I believe. But if you look at the code (marked), it relies on parsing the CDN url, making it a roundabout method of deriving the original values.

private registerSchema() {
    this.editor.model.schema.extend('image', {
        allowAttributes: ['data-upload-path', 'data-upload-bucket'],
    });
}

private registerConverters() {
        const editor = this.editor;

        // Dedicated converter to propagate image's attribute to the img tag.
        editor.conversion.for('downcast').add((dispatcher: any) =>
            dispatcher.on('attribute:src:image', (evt: any, data: any, conversionApi: any) => {
                const viewWriter = conversionApi.writer;

                const figure = conversionApi.mapper.toViewElement(data.item);
                const img = figure.getChild(0);

                if (data.attributeNewValue !== null) {
                    const src = data.attributeNewValue;
                    const url = new URL(src, window.location.origin); 
                    const matches = url.pathname.match(/^\/(.+?)\/(.+)$/); // <-- parsing out the original values from the url
                    const bucket = matches ? matches[1] : '';
                    const path = matches ? matches[2] : '';

                    if (url) {
                        viewWriter.setAttribute('data-upload-bucket', bucket, img);
                        viewWriter.setAttribute('data-upload-key', path, img);
                    }
                } else {
                    viewWriter.removeAttribute('data-upload-bucket', img);
                    viewWriter.removeAttribute('data-upload-key', img);
                }
            })
        );
    }

For an example use case, please see medium.com's CMS. When you upload an image, it inserts a CDN url with resize query parameters. But there are additional attributes that are also set, which can then be used to do other stuff, for example, generating more thumbnails using different size configurations.

what is the actual issue here.

Not an issue. Proposal.

Thanks for your time @Mgsy @oleq !

@oleq
Copy link
Member

oleq commented Sep 23, 2019

@mlewand WDYT?

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-image Oct 9, 2019
@mlewand mlewand added pending:feedback This issue is blocked by necessary feedback. package:image labels Oct 9, 2019
@zrpnr
Copy link

zrpnr commented Oct 29, 2020

If we could have support another attributes property where this information could be passed through, allowing the Custom Upload Adapter to supply any number of attributes to save, that would be very handy.

This would be a nice feature, something similar was also mentioned in #8192.
Currently the object returned from the server needs to match this format:
https://ckeditor.com/docs/ckeditor5/latest/features/image-upload/simple-upload-adapter.html#successful-upload

@darylteo suggestion to allow an attributes property in the server response would be really useful.
It would make it possible to pass in additional information directly from the server about this file, such as a uuid.

{
  url: 'https://example.com/images/foo.jpg',
  attributes: { 
    'data-original-path': 'https://cdn.example.com/foo.jpg',
    'data-uuid': 'abcd',
  },
}

The workaround code here only works if the property can be derived from the path, and won't help for other arbitrary values that the server could provide.

It's still necessary to add any custom attributes in editor.model.schema.extend() like in the workaround, but any attributes besides url or urls in the response are currently discarded before anything is written to the model.

Allowing other attributes in the server response would require a change in both the upload adapter and in the ImageUpload plugin.

For example in the SimpleUploadAdapter,

resolve( response.url ? { default: response.url } : response.urls );

This line would need to be changed to include response.attributes.

And in ImageUpload

model.enqueueChange( 'transparent', writer => {
writer.setAttributes( { uploadStatus: 'complete', src: data.default }, imageElement );
this._parseAndSetSrcsetAttributeOnImage( data, imageElement, writer );
} );

There would need to be another line or helper function here to setAttributes for the new attributes passed in data to actually get it into the model.

@Mgsy Mgsy added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). and removed pending:feedback This issue is blocked by necessary feedback. labels Nov 6, 2020
@Mgsy Mgsy added this to the backlog milestone Nov 6, 2020
@Mgsy Mgsy changed the title [Proposal] Support setting data attributes on img from Custom Upload Adapter Support setting data attributes on image from Custom Upload Adapter Nov 6, 2020
@Mgsy Mgsy changed the title Support setting data attributes on image from Custom Upload Adapter Support setting data attributes on image from upload adapters Nov 6, 2020
@niegowski
Copy link
Contributor

I don't like the idea of an attributes property in the response. Instead, I'd propose to replace the content of this change block

.then( data => {
model.enqueueChange( 'transparent', writer => {
writer.setAttributes( { uploadStatus: 'complete', src: data.default }, imageElement );
this._parseAndSetSrcsetAttributeOnImage( data, imageElement, writer );
} );
with an event fired that would allow integration at this point. So the lines above could look sth like this:

model.enqueueChange( 'transparent', writer => {
	writer.setAttribute( 'uploadStatus', 'complete', imageElement );
	this.fire( 'uploadComplete', { imageElement, data } );
} );

Note that this event would be fired by the ImageUploadEditing plugin (in a similar fashion as ClipboardPipeline is allowing other features to integrate).

I'd also move the src and srcset attributes setting to the dedicated plugins (not sure about). That would look sth like this:

editor.plugins.get( 'ImageUploadEditing' ).on( 'uploadComplete', ( evt, { imageElement, data } ) => {
	editor.model.change( writer => {
		writer.setAttribute( 'src', data.default, imageElement );
	} );
} );

@Reinmar
Copy link
Member

Reinmar commented Mar 29, 2021

Note that this event would be fired by the ImageUploadEditing plugin (in a similar fashion as ClipboardPipeline is allowing other features to integrate).

We discussed this f2f and ClipboardPipeline is part of the essential features. This is a different layer than image upload. I'd say they layer like this:

  • Core, engine, editors, etc.
  • Essential features (e.g. clipboard)
  • Features

The question is whether ImageUploadEditing should know about ImageEditing. It seems that they are in a single package, so it's fine.

One new idea, though, is to move the default listener to uploadComplete to a low priority so default listeners will have precedence over it.

niegowski added a commit that referenced this issue Apr 2, 2021
Feature (image): Introduced the `uploadComplete` event in `ImageUploadEditing` that allows customizing the image element (e.g. setting custom attributes) based on data retrieved from the upload adapter. Closes #5204.

Feature (upload): Upload adapters' async `upload` method can now resolve to an object with additional properties along with the `urls` hash. See more in #5204.

MINOR BREAKING CHANGE (upload): The async `SimpleUploadAdapter#upload()` resolves to an object with normalized data including the `urls` object, which was only returned before. This may affect all integrations depending on the `SimpleUploadAdapter` uploading mechanism.
@Reinmar Reinmar modified the milestones: backlog, iteration 42 Apr 8, 2021
@ghost
Copy link

ghost commented Jul 16, 2021

Hi there, this feature seems like impIemented but I couldn't make it work in anyway. I couldn't find a way to add custom attributes.
Tried this:

editor.plugins.get( 'ImageUploadEditing' ).on( 'uploadComplete', ( evt, { imageElement, data } ) => { editor.model.change( writer => { writer.setAttribute( 'src', data.default, imageElement ); } ); } );
This block works but only on src attribute. I'm also passing the attributes in the object sended from resolve but again no luck. It looks like i need to enable some custom attributes but also tried extending the schema but got an error of you cannot extend a component not registered yet.
Combining it with react is already a pain in the heart unfortunately. I've tried several places to extend the schema to make it work but unfortunately. Any guidance that can really help me?

@ma2ciek
Copy link
Contributor

ma2ciek commented Jul 16, 2021

Hi @datasoysa,

Could you open a new issue as this one is closed?

@testinfected
Copy link

testinfected commented Oct 28, 2021

For anyone interested, it's working fine providing that you extend the imageBlock schema.

@datasoysa in your example {imageElement, data}, should be {data, imageElement}

Here's some code that works for me:

editor.plugins.get( 'ImageUploadEditing' ).on( 'uploadComplete', ( evt, { data, imageElement } ) => {      
       editor.model.change( writer => {
           writer.setAttribute( 'dataReference', data.reference, imageElement );
       } ); } ); 
                        
        editor.model.schema.extend( 'imageBlock', { allowAttributes: 'dataReference' } );

        editor.conversion.for( 'upcast' ).attributeToAttribute( {
            view: 'data-reference',
            model: 'dataReference'
        } );

        editor.conversion.for( 'downcast' ).add( dispatcher => {
            dispatcher.on( 'attribute:dataReference:imageBlock', ( evt, data, conversionApi ) => {
                if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
                    return;
                }

                const viewWriter = conversionApi.writer;
                const figure = conversionApi.mapper.toViewElement( data.item );
                const img = figure.getChild( 0 );

                if ( data.attributeNewValue !== null ) {
                    viewWriter.setAttribute( 'data-reference', data.attributeNewValue, img );
                } else {
                    viewWriter.removeAttribute( 'data-reference', img );
                }
            } );
        } );      

@solilin
Copy link

solilin commented Aug 16, 2023

@testinfected could you help me, your code example works pretty well, but I have an issue with the edit form. When I have a with data-id='352' in my content, it seems that CKEditor cut it.

@Witoso
Copy link
Member

Witoso commented Aug 16, 2023

@solilin most likely you need to swap the 'data-reference' with the 'data-id' as you use a different attribute name.

@solilin
Copy link

solilin commented Aug 16, 2023

no, in my case, I have data-id instead of data-reference

@testinfected
Copy link

testinfected commented Aug 16, 2023

@solilin It’s been a long time, I don’t remember the details. Maybe CKEditor treats data-id as a special attribute. That might be why I settled on data-reference

@Witoso
Copy link
Member

Witoso commented Aug 17, 2023

but I have an issue with the edit form.

Could you elaborate @solilin?

@solilin
Copy link

solilin commented Aug 17, 2023

@Witoso I have a scenario where a user can both create and edit a post using the CKEditor content field. The above code works perfectly for creating posts. When a user uploads images, the required data-id for each image is present. However, a problem arises when a user opens an existing post with images for editing. During this process, the imageBlock loses its data-id, indicating a possible issue with the functioning of the upcast feature.

@Witoso
Copy link
Member

Witoso commented Aug 18, 2023

Gotcha, most likely because the snippet above only extends the schema after image upload:

editor.plugins.get( 'ImageUploadEditing' ).on( 'uploadComplete', ( evt, { data, imageElement } ) => {
// 						^^^^^^^^^ 
// 					Extending happens only on uploadComplete
}

When the content is loaded, the editor doesn't know this attribute can be stored on an image.

What you need to do is to create your plugin and add it to the editor so that it works in every case. With a quick glance, copying and pasting should work:

function imageDataIdPlugin( editor ) {
                        
        editor.model.schema.extend( 'imageBlock', { allowAttributes: 'dataId' } );

        editor.conversion.for( 'upcast' ).attributeToAttribute( {
            view: 'data-id',
            model: 'dataId'
        } );

        editor.conversion.for( 'downcast' ).add( dispatcher => {
            dispatcher.on( 'attribute:dataReference:imageBlock', ( evt, data, conversionApi ) => {
                if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
                    return;
                }

                const viewWriter = conversionApi.writer;
                const figure = conversionApi.mapper.toViewElement( data.item );
                const img = figure.getChild( 0 );

                if ( data.attributeNewValue !== null ) {
                    viewWriter.setAttribute( 'data-id', data.attributeNewValue, img );
                } else {
                    viewWriter.removeAttribute( 'data-id', img );
                }
            } );
}

Add it to the plugins lists of the editor, and in the .on( 'uploadComplete', ... only set the attribute.

@solilin
Copy link

solilin commented Aug 18, 2023

It works, @Witoso you’re a lifesaver. I spent almost 1 day with this problem.

@godofevil
Copy link

Hi guys, I get totally stuck in same issue, I'm trying the upper solution, but got
CKEditorError: schema-cannot-extend-missing-item {"itemName":"imageBlock"}

@Witoso
Copy link
Member

Witoso commented Nov 3, 2023

@godofevil could you share your plugins lists, looks like Image plugin is not loaded (or ImageBlock).

@LucasHrqc
Copy link

LucasHrqc commented Dec 14, 2023

@Witoso Does this work if any text is also typed? I've used and it worked, but only when the ckeditor contains only the image. As soon as I type anything and try uploading it, the data-attribute is not set.

The ImageAttribute function contains exactly what you posted above. Not currently using it as a plugin.

Editor.ClassicEditor.create(
          document.querySelector("#input_" + this.input.alias),
          this.config
        )
          .then((editor) => {

            // ------- Sets editor's height -----------
            editor.editing.view.change((writer) => {
              let height = this.input.height ?? "280px";
              writer.setStyle("height", height, editor.editing.view.document.getRoot());
              writer.setStyle("color", "#71748d", editor.editing.view.document.getRoot());
            });

            // Hides the toolbar when in read only mode.
            const toolbarElement = editor.ui.view.toolbar.element;
            editor.on("change:isReadOnly", (evt, propertyName, isReadOnly) => {
              if (isReadOnly) {
                toolbarElement.style.display = "none";
              } else {
                toolbarElement.style.display = "flex";
              }
            });

            // ------- Sets editor's content when it changes -----------
            editor.model.document.on("change:data", () => {
              this.formData[this.input.name] = editor.getData();
            });

            // ------- Deals with midia upload -----------
            editor.plugins.get("FileRepository").createUploadAdapter = (loader) => {
              return new CKEditorUploadAdapter(loader, this.$route.params.route);
            };

            ImageAttributes(editor);

            // ------- Make editor's instance available after mounted -----------
            window[this.input.alias + "_editor"] = editor;
          })
          .catch((err) => {
            console.error(err.stack);
            this.$notify({
              type: "error",
              text: "Houve algum erro no carregamento do formulário.",
            });
          });

EDIT:
Solved it by doing the same for imageInline, except that imageInline doens't have the figure element. Plugin:

import { Plugin } from "ckeditor5-custom-build/build/ckeditor";

export default class ImgIdAttributePlugin extends Plugin {
  init() {
    const editor = this.editor;

    editor.plugins.get("ImageUploadEditing").on("uploadComplete", (evt, { data, imageElement }) => {
      editor.model.change((writer) => {
        writer.setAttribute("dataFileid", data.fileid, imageElement);
      });
    });

    editor.model.schema.extend("imageBlock", { allowAttributes: "dataFileid" });
    editor.model.schema.extend("imageInline", { allowAttributes: "dataFileid" });

    editor.conversion.for("upcast").attributeToAttribute({
      view: "data-fileid",
      model: "dataFileid",
    });

    editor.conversion.for("downcast").add((dispatcher) => {
      dispatcher.on("attribute:dataFileid:imageBlock", (evt, data, conversionApi) => {
        this._setAttribute(evt, data, conversionApi, "imageBlock");
      });
      dispatcher.on("attribute:dataFileid:imageInline", (evt, data, conversionApi) => {
        this._setAttribute(evt, data, conversionApi, "imageInline");
      });
    });
  }

  _setAttribute(evt, data, conversionApi, type) {
    if (!conversionApi.consumable.consume(data.item, evt.name)) {
      return;
    }

    const viewWriter = conversionApi.writer;
    let figure = null;
    let img = conversionApi.mapper.toViewElement(data.item);
    if (type == "imageBlock") {
      figure = img;
      img = figure.getChild(0);
    }

    if (data.attributeNewValue !== null) {
      viewWriter.setAttribute("data-fileid", data.attributeNewValue, img);
    } else {
      viewWriter.removeAttribute("data-fileid", img);
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:image type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.