Opened 8 years ago

Closed 8 years ago

#4358 closed New Feature (fixed)

List properties dialog box is missing

Reported by: Wiktor Walc Owned by: Minh Nguyen
Priority: Normal Milestone: CKEditor 3.3
Component: Core : Lists Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc: michel.denys@…

Description

The properties for bulleted/numbered lists are missing (this feature is available in FCKeditor: when you click on a list, a "Bulleted List Properties" / "Numbered List Properties" item is available in the context menu).

The issue was reported here: http://cksource.com/forums/viewtopic.php?f=6&t=14342

Attachments (8)

fck-list-properties.png (3.4 KB) - added by Denys 8 years ago.
liststyle_ref.zip (1.9 KB) - added by Minh Nguyen 8 years ago.
4358.patch (7.6 KB) - added by Minh Nguyen 8 years ago.
4358_2.patch (8.4 KB) - added by Minh Nguyen 8 years ago.
4358_3.patch (9.3 KB) - added by Minh Nguyen 8 years ago.
4358_4.patch (9.4 KB) - added by Minh Nguyen 8 years ago.
4358_5.patch (9.2 KB) - added by Minh Nguyen 8 years ago.
Thank Garry, I have got your point.
4358_7.patch (9.2 KB) - added by Minh Nguyen 8 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 8 years ago by Denys

You can view the missing bullet properties @ http://ckeditor.com/demo

When do you think this could be fixed? Is there a workaround? I need the speed of CKeditor but without the list properties I have to stay with FCKeditor?

comment:2 Changed 8 years ago by Denys

Component: GeneralCore : Lists
Version: 3.0

Changed 8 years ago by Denys

Attachment: fck-list-properties.png added

comment:3 Changed 8 years ago by Denys

Print-screen of FCKeditor function:

comment:4 Changed 8 years ago by Denys

Cc: michel.denys@… added

comment:5 Changed 8 years ago by Garry Yao

Keywords: Confirmed removed
Milestone: CKEditor 3.1CKEditor 3.2
Version: 3.0SVN (CKEditor)

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

comment:7 Changed 8 years ago by Denys

Possible workaround:

In the Style combo plugin (plugins\stylescombo\default.js), add an attribute for the ol element (or the ul element). For example, for roman lists:

CKEDITOR.addStylesSet('default',[
{name :'Roman',element:'ol',attributes:{type:'I'}},
]);

It works BUT to apply the style, the ol element must be selected. The only method I saw is to select it via the name of the element shown at the bottom of the editor. Not very user friendly for casual users. (see the CKEDITOR FAQ http://cksource.com/forums/viewtopic.php?f=11&t=17301)

comment:8 Changed 8 years ago by Garry Yao

Keywords: Confirmed added

Thanks kurgbe, we should be able to align this feature with v2 in this milestone, the styles combo is not sufficient to carry out this feature, we still need a classical dialog here.

comment:9 Changed 8 years ago by Minh Nguyen

Owner: set to Minh Nguyen
Status: newassigned

Changed 8 years ago by Minh Nguyen

Attachment: liststyle_ref.zip added

Changed 8 years ago by Minh Nguyen

Attachment: 4358.patch added

comment:10 Changed 8 years ago by Minh Nguyen

Keywords: Review? added

comment:11 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Alternate bulleted list type in IE doesn't work (it should be circle instead of "Circle"), besides, in V3 we should deprecate the "type" attribute in favor of list-style-type CSS property.
Also please use "<not set>" for unknown list type instead of leaving it empty.
Besides, Can we make the Bulleted List Type field a bit wider, make it looks better?

Changed 8 years ago by Minh Nguyen

Attachment: 4358_2.patch added

comment:12 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:13 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Please use "<not set>" instead of 'inherit' as display option, which is more user-friendly, it's enough to just delete the style when "<not set>" is selected. Also this option is missing from the Bulleted List type.

comment:14 Changed 8 years ago by Garry Yao

Also note that while adapting to the CSS style, we should still provide compatibility to the "type" attribute which make comes from content produced by v2.

comment:15 Changed 8 years ago by Denys

Thanks for the patch. Suggestion: instead of <not set> or <Inherit> we could use <Default>

Changed 8 years ago by Minh Nguyen

Attachment: 4358_3.patch added

comment:16 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:17 Changed 8 years ago by Minh Nguyen

I using '<not set>' for user doesn't select type of list.

comment:18 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Finally, we need a mapping between attribute type and list-style-type.
Also, it's enough to use empty string for 'notset'.

Changed 8 years ago by Minh Nguyen

Attachment: 4358_4.patch added

comment:19 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:20 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

That's great, but I think you misunderstood my points that the mapping should be applied to the 'setup' of list type instead of on 'commit':

this.setValue( element.getStyle( 'list-style-type' ) 
        || mapListStyle[ element.getAttribute( 'type' ) ] 
        || element.getAttribute( 'type' ) 
        || '' );

Changed 8 years ago by Minh Nguyen

Attachment: 4358_5.patch added

Thank Garry, I have got your point.

comment:21 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:22 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

There's a problem with the current approach(also v2), with the following list, it's not able to recognize the list type properly.

<ul type="i">
	<li>item</li>
</ul>

Also, I can't get the 'start' field working.

Changed 8 years ago by Minh Nguyen

Attachment: 4358_7.patch added

comment:23 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:24 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

Be sure the following thing will happenen before committing:

  1. Register plugin into core config in alphabetic order;
  2. Run lang tool to update all other language files;

comment:25 Changed 8 years ago by Minh Nguyen

Resolution: fixed
Status: assignedclosed

Fixed with [5409].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy