Labels don't fit the Table Properties dialog in some languages.

[j.swiderski]

In [6979] unit select list has been removed. Now ,for some languages, the labels describing cell padding and cell spacing don’t fit the dialog.

These languages are: Vietnamese, Russian, Lithuanian, Georgian, French, Faroese, Dutch, Czech, Catalan, Bulgarian, Mongolian, Polish

In Firefox and Opera labels go out of dialog. In IE9 part of label is hidden (like overflow:hidden) In Webkit and IE8 scroll bars appear.

In IE6 and IE7 dialog is enlarged to fit the label. In other words it looks nice here.

[garry.yao]

Can't we just resigns all dimension restrictions over the table dialog, which makes l11n just easier and also looks neater.

[Saare]

Looks good to me, if anyone sees a problem please reassign the status of the ticket.

[wwalc]

In my opinion the proposed solution is not right if we think of user experience.

We should keep input fields short where we don't expect large numbers.

[garry.yao]

Right now we had a limitation in the dialog layout that the specified field width is bound with label size, which means short field won't allow a long label fill to stretch the dialog. E.g. having the following definition:

 type : 'text',
 id : 'txtCellSpace',
 style : 'width:3em'

Constraint both the label and input to "3em" width.

Propose a new "inputStyle" definition entry which makes it possible to have fine-grid control over style on both input element and the label.

[alfonsoml]

There's already a "controlStyle" option: ​http://dev.ckeditor.com/ticket/5927

[Saare]

Replying to alfonsoml:

There's already a "controlStyle" option: ​http://dev.ckeditor.com/ticket/5927

That's different, as controlStyle is applied to the input itself while inputStyle is applied to the surrounding <div>. Though we really are missing the docs for controlStyle.

[alfonsoml]

It's gonna be fun to have "inputStyle" applied to a div and "controlStyle" applied to an input.

Are we sure that we want to use those names?
Once it's released we'll be again on the old game "yeah, that's bad, but changing it would break external code. We might change it in the next big rewrite 2 years from now"

[wwalc]

Indeed it might be quite confusing... The patch is good, but using better names for options right from the beginning also sounds reasonable. Alfonso, do you have any suggestions?

[garry.yao]

Replying to alfonsoml:

It's gonna be fun to have "inputStyle" applied to a div and "controlStyle" applied to an input.

As you proposed "controlStyle" back in #5927 to introduce an easy way of styling the control element separately from label, exactly of the same purpose with me here (but I didn't know that option and it turns out to have only one usage in the codebase).

I propose to deprecate the "controlStyle" in favor of "inputStyle" for the following reasons:

1. The style should goes on the input wrapper (e.g. cke_dialog_ui_input_text) instead of the input element itself, as the wrapper carries part of the input style.
2. IMO "inputStyle" get also better adopted to the current code as we already have something like "getInputElement".

[garry.yao]

@Saar, can you review the changes to the docProps dialog if it breaks anything?

[wwalc]

Using the latest patch, page margins in the "Design" tab in the docProps dialog are no longer centered (to reproduce just type something in the "Top" field).

[garry.yao]

I didn't realize the input field content should also be center aligned...but that would be quite a rare case to be considered.

[garry.yao]

illustration of the dialog field width definition

[garry.yao]

Just to better illustrate the actual effects of those definitions:

[wwalc]

DocProps dialog: actually I do not see any reason why

this.getElement().getParent().setStyle( 'text-align', 'center' ); 

was used there. Am I wrong or controlStyle : 'text-align: center' was enough to keep the text aligned to the center?

Regarding controlStyle / labelStyle / inputStyle (I'll use current names in this post just to avoid confusion). In my opinion they are all useful. Thanks to the controlStyle I believe we can get rid of

 function textCenterAlign() { this.getInputElement().setStyle( 'text-align', 'center' ); } 

used in the last patch. Being able to style the element perfectly using just the definition I believe is something that Alfonso tried to achieve with #5927.

Alfonso suggested using better names, although I agree that we should not use confusing names, I have no idea what names would be better :|.

One more thing regarding the functionality, I've just made a quick test and noticed some inconsistency (using the 2# patch if I'm correct).

{
	type: 'text',
	id : 'foo1',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	default : 'initial value',
	labelStyle : 'color:red'
},
{
	type: 'textarea',
	id : 'foo2',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	default : 'initial value',
	labelStyle : 'color:red'
},
{
	type: 'select',
	id : 'foo3',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	labelStyle : 'color:red',
	items : [ ['foo'] ]
},
{
	type: 'checkbox',
	id : 'foo4',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	labelStyle : 'color:red'
},
{
	type: 'radio',
	id : 'foo5',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	labelStyle : 'color:red',
	items : [ ['foo'] ]
},
{
	type: 'html',
	id : 'foo6',
	style : 'width:200px',
	html : 'foo html',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue'
},
{
	id : 'foo7',
	type: 'file',
	style : 'width:200px',
	label : 'label',
	inputStyle : 'border:5px solid red',
	controlStyle : 'color:blue',
	labelStyle : 'color:red'
}

[garry.yao]

Following Wiktor's review, switch the names between 'inputStyle' and 'controlStyle' for better semantics, fixing the above mentioned bugs.

[alfonsoml]

Another option (just in case you want another idea, but I think that the latest patch is quite better to keep everyone's sanity than the original proposal) would be to call the style for the wrapper div inputWrapperStyle instead of controlStyle.

When the dialog system is reviewed, it should be checked if such div wrapper is really needed or it can be removed, less elements, less things to think about.

[wwalc]

I've noticed that in fact we're missing the documentation for CKEDITOR.dialog.definition.labeledElement. The attached patch contains changes from Garry's patch to dialogDefinition.js along with some further improvements.

[Saare]

The code pieces in the DocProps plugin are there to align the fields in Quirks mode, thus it must remain untoucned.
It's otherwise R+, so I'd opt to commit the patch without these changes (let's also add a comment there to remind what's the purpose of this code piece).

[garry.yao]

Fixed with [7029].