Opened 10 years ago

Closed 8 years ago

#635 closed New Feature (fixed)

Open properties dialog when double clicking on objects

Reported by: Frederico Caldeira Knabben Owned by: Minh Nguyen
Priority: Normal Milestone: CKEditor 3.3
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

The relative dialog should open when double clicking on objects like images, anchors, flash, form buttons or even tables (in the border... IE only I guess).

Attachments (9)

635.patch (1.4 KB) - added by Artur Formella 9 years ago.
635_2.patch (1.5 KB) - added by Minh Nguyen 8 years ago.
635_3.patch (10.2 KB) - added by Minh Nguyen 8 years ago.
635_4.patch (2.1 KB) - added by Minh Nguyen 8 years ago.
New patch with simple detection
653_ref.patch (1.8 KB) - added by Garry Yao 8 years ago.
635_5.patch (2.5 KB) - added by Minh Nguyen 8 years ago.
635_6.patch (4.7 KB) - added by Minh Nguyen 8 years ago.
635_7.patch (5.3 KB) - added by Minh Nguyen 8 years ago.
add missing flash detection
635_8.patch (4.9 KB) - added by Minh Nguyen 8 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 10 years ago by Wojciech Olchawa

Keywords: Confirmed added

comment:2 Changed 9 years ago by Artur Formella

Owner: set to Artur Formella
Status: newassigned

Changed 9 years ago by Artur Formella

Attachment: 635.patch added

comment:3 Changed 9 years ago by Artur Formella

Keywords: Review? added

This is only an idea + working example. Is it good way?

comment:4 Changed 9 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.1

comment:5 Changed 8 years ago by Artur Formella

Owner: Artur Formella deleted
Status: assignednew

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Milestone: CKEditor 3.1CKEditor 3.2

The idea is good. But of course, the patch is still quite incomplete, as we need to enable it for all kinds of objects we support.

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

Changed 8 years ago by Minh Nguyen

Attachment: 635_2.patch added

comment:8 Changed 8 years ago by Minh Nguyen

Owner: set to Minh Nguyen
Status: newassigned

comment:9 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:10 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

I like the idea, but it will give an error if the contextMenu plugin isn't available.

Changed 8 years ago by Minh Nguyen

Attachment: 635_3.patch added

comment:11 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:12 Changed 8 years ago by Alfonso Martínez de Lizarrondo

With my previous comment I just expected to protect the reading of editor.contextMenu._.listeners because that solution was clever and didn't mean extra work.

This new patch means that it's possible to use this feature even if the contextmenu plugin is removed, but I'm not sure if it's worth the extra work. I would like to hear the opinions from someone else.

A benefit of this approach is that there won't be problems if any extra context menu is added.

comment:13 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

The behavior on 'dbclick' should be distinguished from which on 'contextmenu', e.g. 635_2.patch opens table dialog when double clicking inside cell, which is wrong.
Beside, the detection mechanism should be simplified, where dialog should simply declare a listener of one param - element, and it would be better the 'wysiwyg' plugin's responsibility to hold such listeners instead of 'dialog' plugiin.

Changed 8 years ago by Minh Nguyen

Attachment: 635_4.patch added

New patch with simple detection

comment:14 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

Changed 8 years ago by Garry Yao

Attachment: 653_ref.patch added

comment:15 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

There's a misunderstanding here Minh, with 635_4.patch, other dialogs are not able to register to this behavior, perhaps it's better to demonstrate my idea with a patch.
Beside, considering that block element like table/form are just hard to get double-clicked, we should just leave them for now and proposing some other usability feature to achieve the goal, e.g. by double clicking elements path bar item, but it's out the scope of this ticket.

Changed 8 years ago by Minh Nguyen

Attachment: 635_5.patch added

comment:16 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:17 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

It's not good for image plugin to handle all fake elements' dialog, instead it should be the original dialog, e.g. anchor, flash to detect the fake element.

if ( element.is( 'img' ) )
 evt.data.dialog =  realElement ? realElement : 'image';

Besides, other dialogs like form fields are missing from the patch.

Changed 8 years ago by Minh Nguyen

Attachment: 635_6.patch added

comment:18 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:19 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Flash dialog is still missing.

Changed 8 years ago by Minh Nguyen

Attachment: 635_7.patch added

add missing flash detection

comment:20 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:21 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

The patch couldn't be applied on 3.3.x branch where it should be committed, please include the 'hiddenfield' into it also and remove the table cell registration, since it's quite often that people click inside table cell just to highlight a word.

comment:22 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

ticket #4056 had resolved problem with 'hiddenfield'.

Changed 8 years ago by Minh Nguyen

Attachment: 635_8.patch added

comment:23 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

This feature should go into 3.3.x branch when committing.

comment:24 Changed 8 years ago by Minh Nguyen

Resolution: fixed
Status: assignedclosed

Fixed with [5378].

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