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

Setting a class attribute on model elements breaks Widget handling #6601

Closed
rshipp opened this issue Apr 13, 2020 · 4 comments
Closed

Setting a class attribute on model elements breaks Widget handling #6601

rshipp opened this issue Apr 13, 2020 · 4 comments
Labels
domain:dx This issue reports a developer experience problem or possible improvement. package:engine resolution:expired This issue was closed due to lack of feedback. status:stale type:bug This issue reports a buggy (incorrect) behavior.

Comments

@rshipp
Copy link
Contributor

rshipp commented Apr 13, 2020

📝

If you set a class attribute on a model element, then try to use toWidget or toWidgetEditable in the editingDowncast converter, the ck-widget class will be removed before the element is rendered in the editing view.

It took me a long time to figure this out, so I'd like to make sure it gets documented somewhere (assuming this is not a bug).

Example:

    // This breaks!
    conversion.for( 'upcast' ).elementToElement( {
        view: {
            name: 'div',
            classes: ['my-element']
        },
        model: ( viewElement, modelWriter ) => {
            return modelWriter.createElement( 'myElement', {
                'class': viewElement.getAttribute('class') || 'my-element',
            });
        }
    } );
    conversion.for( 'editingDowncast' ).elementToElement( {
        model: 'myElement',
        view: ( modelElement, viewWriter ) => {
            const moduleBlock = viewWriter.createContainerElement( 'div', {
                'class': modelElement.getAttribute('class') || 'my-element',
            } );

            return toWidget( myElement, viewWriter, { label: 'my-element widget' } );
        }
    } );
    // This works.
    conversion.for( 'upcast' ).elementToElement( {
        view: {
            name: 'div',
            classes: ['my-element']
        },
        model: ( viewElement, modelWriter ) => {
            return modelWriter.createElement( 'myElement', {
                'preserveClasses': viewElement.getAttribute('class') || 'my-element',
            });
        }
    } );
    conversion.for( 'editingDowncast' ).elementToElement( {
        model: 'myElement',
        view: ( modelElement, viewWriter ) => {
            const moduleBlock = viewWriter.createContainerElement( 'div', {
                'class': modelElement.getAttribute('preserveClasses') || 'my-element',
            } );

            return toWidget( myElement, viewWriter, { label: 'my-element widget' } );
        }
    } );

I'm happy to do a documentation update PR, but I'm not sure where is a good place to put this information. Any ideas?

@rshipp rshipp added the type:docs This issue reports a task related to documentation (e.g. an idea for a guide). label Apr 13, 2020
@Mgsy Mgsy added the pending:feedback This issue is blocked by necessary feedback. label Apr 14, 2020
@Mgsy
Copy link
Member

Mgsy commented Apr 14, 2020

cc @Reinmar

@Reinmar
Copy link
Member

Reinmar commented Apr 14, 2020

I think we stumbled upon this or a similar problem already. I don't remember, though, what was causing this problem. I found two similar issues: #4517 and #3795 for the style attribute. However, AFAIR the problem was with the view, not the model. I'm curious if in the scenario that @rshipp reported the problem lays indeed in the model or somewhere else. 

But one thing's for sure – it's definitely a bug. IMO, such things should work as one expects.

@Reinmar Reinmar added type:bug This issue reports a buggy (incorrect) behavior. and removed type:docs This issue reports a task related to documentation (e.g. an idea for a guide). labels Apr 14, 2020
@Reinmar Reinmar added this to the nice-to-have milestone Apr 14, 2020
@Reinmar Reinmar added domain:dx This issue reports a developer experience problem or possible improvement. package:engine and removed pending:feedback This issue is blocked by necessary feedback. labels Apr 14, 2020
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 12, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dx This issue reports a developer experience problem or possible improvement. package:engine resolution:expired This issue was closed due to lack of feedback. status:stale type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

5 participants