Opened 9 years ago

Closed 7 years ago

#635 closed New Feature (fixed)

Open properties dialog when double clicking on objects

Reported by: fredck Owned by: m.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 arczi 8 years ago.
635_2.patch (1.5 KB) - added by m.nguyen 7 years ago.
635_3.patch (10.2 KB) - added by m.nguyen 7 years ago.
635_4.patch (2.1 KB) - added by m.nguyen 7 years ago.
New patch with simple detection
653_ref.patch (1.8 KB) - added by garry.yao 7 years ago.
635_5.patch (2.5 KB) - added by m.nguyen 7 years ago.
635_6.patch (4.7 KB) - added by m.nguyen 7 years ago.
635_7.patch (5.3 KB) - added by m.nguyen 7 years ago.
add missing flash detection
635_8.patch (4.9 KB) - added by m.nguyen 7 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 9 years ago by w.olchawa

  • Keywords Confirmed added

comment:2 Changed 8 years ago by arczi

  • Owner set to arczi
  • Status changed from new to assigned

Changed 8 years ago by arczi

comment:3 Changed 8 years ago by arczi

  • Keywords Review? added

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

comment:4 Changed 8 years ago by fredck

  • Milestone changed from CKEditor 3.0 to CKEditor 3.1

comment:5 Changed 7 years ago by arczi

  • Owner arczi deleted
  • Status changed from assigned to new

comment:6 Changed 7 years ago by fredck

  • Keywords Review- added; Review? removed
  • Milestone changed from CKEditor 3.1 to CKEditor 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 7 years ago by fredck

  • Milestone changed from CKEditor 3.2 to CKEditor 3.3

Changed 7 years ago by m.nguyen

comment:8 Changed 7 years ago by m.nguyen

  • Owner set to m.nguyen
  • Status changed from new to assigned

comment:9 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:10 Changed 7 years ago by alfonsoml

  • Keywords Review- added; Review? removed

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

Changed 7 years ago by m.nguyen

comment:11 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:12 Changed 7 years ago by alfonsoml

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 7 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 7 years ago by m.nguyen

New patch with simple detection

comment:14 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

Changed 7 years ago by garry.yao

comment:15 Changed 7 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 7 years ago by m.nguyen

comment:16 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:17 Changed 7 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 7 years ago by m.nguyen

comment:18 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:19 Changed 7 years ago by garry.yao

  • Keywords Review- added; Review? removed

Flash dialog is still missing.

Changed 7 years ago by m.nguyen

add missing flash detection

comment:20 Changed 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:21 Changed 7 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 7 years ago by m.nguyen

  • Keywords Review? added; Review- removed

ticket #4056 had resolved problem with 'hiddenfield'.

Changed 7 years ago by m.nguyen

comment:23 Changed 7 years ago by garry.yao

  • Keywords Review+ added; Review? removed

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

comment:24 Changed 7 years ago by m.nguyen

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [5378].

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