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

Is it possible to set style attribute on td and `table tag ? #3235

Closed
gauravjain028 opened this issue Sep 24, 2018 · 13 comments
Closed

Is it possible to set style attribute on td and `table tag ? #3235

gauravjain028 opened this issue Sep 24, 2018 · 13 comments
Labels
package:table pending:feedback This issue is blocked by necessary feedback. resolution:duplicate This issue is a duplicate of another issue and was merged into it. type:bug This issue reports a buggy (incorrect) behavior. type:question This issue asks a question (how to...).

Comments

@gauravjain028
Copy link

I have created color plugins which works fine on $text and $block. It can select any color from color picker and set that color as style="color:#AAAAAA;" .

Now I want to do like, if I select the whole table by it's handler, and can select the color. After that it can update the border color as well. Currently it updated the color of text written inside.

It is possible ?

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2018

Hi! It simply depends on where you apply that attribute. If you apply it to a text and write a converter for a text attribute, then it's displayed on the text. If you feature will apply it to a <table> or <tableCell> elements and you'll use downcastAttributeToAttribute() (you need upcastAttributeToAttribute() too, of course), then this attribute will appear on the respective view elements.

Note: Adding support for classes that are set on <figure class="table *"> is much easier. See #1462.

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2018

LOL, I need to take a screenshot of what I posted above for the future :D I'll remove the comment cause the code couldn't work in any universe:

image

Obviously, downcast/upcastAttrToAttr() need to be called with editor.conversion.for().add() :D

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2018

OK, this is an awful day. The corrected code:

editor.model.schema.extend( 'table', {
	allowAttributes: 'style'
} );

editor.conversion.for( 'upcast' ).add( upcastAttributeToAttribute( {
	view: {
		name: 'table',
		key: 'style'
	},
	model: 'style'
} ) );

editor.conversion.for( 'downcast' ).add( downcastAttributeToAttribute( {
	model: {
		name: 'table',
		key: 'style'
	},
	view: 'style'
} ) );

window.setTableStyle = function( style ) {
	const table = editor.model.document.selection.getFirstPosition().getAncestors().find( item => item.name == 'table' );

	editor.model.change( writer => {
		writer.setAttribute( 'style', style, table );
	} );
};

But what the hell is this:

image

There are two issues:

  1. That this attribute gets assigned to the <figure>. Now, this is understandable when you know the structure of the view, but how to write a converter which will apply that attribute to a <table>?

  2. Why was the value of this attribute passed via some Array.from()? Did I make some mistake or is it a bug?

    image

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2018

cc @jodator @scofalik

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2018

BTW, I also found the API confusing. I realised that the mistake I did is something that may happen to anyone simply because I was just copying pieces of code from API docs. This matter is being discussed in https://github.com/ckeditor/ckeditor5-engine/issues/1556#issuecomment-424105185

@gauravjain028
Copy link
Author

@Reinmar Thanks for explanation. I have a question, from where should I call setTableStyle ?

@Reinmar
Copy link
Member

Reinmar commented Sep 25, 2018

From wherever you want :) That's just an example code which applies an attribute on a model's table element. Depending on your use case you need to turn it into some command. You said that you want it to work when a table is selected, so I'd recommend using Selection#getSelectedElement().

@gauravjain028
Copy link
Author

Thanks. I got it working. But having same issue as you had

That this attribute gets assigned to the <figure>. Now, this is understandable when you know the structure of the view, but how to write a converter which will apply that attribute to a <table>?

Is there any workaround for it or it is going to be fixed in ckeditor?

@Reinmar
Copy link
Member

Reinmar commented Sep 25, 2018

I hope that @jodator or @scofalik will be able to provide some quick and easy workaround now. If not, then we need to improve those converter factories I showed you above.

@Reinmar
Copy link
Member

Reinmar commented Sep 25, 2018

PS. that's not a bug. The model's <table> is represented by the <figure> in the view. But we need to make it simple to map model <table>'s attr to <table>'s attr in the view.

@gauravjain028
Copy link
Author

@Reinmar @jodator @scofalik
Waiting for your reply, please help me on this.

@jodator jodator self-assigned this Oct 4, 2018
@jodator
Copy link
Contributor

jodator commented Oct 4, 2018

@gauravjain028: Sorry for longer response - we have a release in progress so we have less time to answer questions.

So the working code for styling background-color style is:

editor.model.schema.extend( 'table', {
	allowAttributes: 'style'
} );

editor.conversion.for( 'upcast' ).add( upcastAttributeToAttribute( {
	view: {
		name: 'table',
		key: 'style',
		value: {
			'background-color': /[\s\S]+/
		}
	},
	model: {
		key: 'style',
		value: viewElement => viewElement.getStyle( 'background-color' )
	}
} ) );

editor.conversion.for( 'downcast' ).add( dispatcher => {
	dispatcher.on( 'attribute:style:table', ( evt, data, conversionApi ) => {
		const table = data.item;

		// The table from the model is mapped to the widget element: <figure>.
		const viewFigure = conversionApi.mapper.toViewElement( table );

		// A <table> is direct child of a <figure> but there might be other view (including UI) elments inside <figure>.
		const viewTable = [ ...viewFigure.getChildren() ].find( element => element.name === 'table' );

		// it should be consumed...

		// User view writer to change style of a view table.
		if ( data.attributeNewValue ) {
			conversionApi.writer.setStyle( 'background-color', data.attributeNewValue, viewTable );
		} else {
			conversionApi.writer.removeStyle( 'background-color', viewTable );
		}
	} );
} );

window.setTableStyle = function( style ) {
	const table = editor.model.document.selection.getFirstPosition().getAncestors().find( item => item.name == 'table' );

	editor.model.change( writer => {
		writer.setAttribute( 'style', style, table );
	} );
};

There are few things to note:

  1. AFAICS you cannot just use style with conversion helpers - or at least I was unable to do so (cc @scofalik?)
  2. Downcasting attributes on widgets is trouble some because model element is mapped to <figure> not the feature element (ie table, image).
  3. Allowing any element as an model value is not so good documented (or I was unable to quickly find them)

cc @Reinmar - look on those remarks as we might think how to improve that for widgets. (see linked issue from Image feature).

@Reinmar Reinmar assigned Reinmar and unassigned jodator Apr 26, 2019
@Reinmar
Copy link
Member

Reinmar commented May 27, 2019

I reported a ticket for the bug when you try to use attributeToAttribute() converter for style: https://github.com/ckeditor/ckeditor5-engine/issues/1740. There's a workaround mentioned there how to allow style on <td> or <tr>.

Regarding allowing style on <table> and <image> there's an example in https://github.com/ckeditor/ckeditor5-core/compare/poc/customfigureattributes which does that. It will end up one day in the documentation.

@Reinmar Reinmar closed this as completed May 27, 2019
@Reinmar Reinmar removed their assignment May 27, 2019
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-table Oct 9, 2019
@mlewand mlewand added pending:feedback This issue is blocked by necessary feedback. resolution:duplicate This issue is a duplicate of another issue and was merged into it. type:bug This issue reports a buggy (incorrect) behavior. type:question This issue asks a question (how to...). package:table labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table pending:feedback This issue is blocked by necessary feedback. resolution:duplicate This issue is a duplicate of another issue and was merged into it. type:bug This issue reports a buggy (incorrect) behavior. type:question This issue asks a question (how to...).
Projects
None yet
Development

No branches or pull requests

4 participants