Opened 16 years ago

Closed 15 years ago

#4048 closed Bug (fixed)

Context submenu lacks uiColor

Reported by: Tobiasz Cudnik Owned by: Tobiasz Cudnik
Priority: Normal Milestone: CKEditor 3.1
Component: General Version:
Keywords: Kama Confirmed Review+ Cc:

Description

Context submenu lacks uiColor in Kama skin.

Attachments (3)

4048.patch (5.3 KB) - added by Tobiasz Cudnik 16 years ago.
4048_2.patch (7.1 KB) - added by Tobiasz Cudnik 16 years ago.
4048_3.patch (7.7 KB) - added by Tobiasz Cudnik 15 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 16 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

Changed 16 years ago by Tobiasz Cudnik

Attachment: 4048.patch added

comment:2 Changed 16 years ago by Tobiasz Cudnik

Keywords: Review? added

comment:3 Changed 16 years ago by Frederico Caldeira Knabben

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

These changes are a bit risky at this point. Let's delay it a bit.

In any case, i think we should find a solution where the other plugins can listen for the uiColor changes somehow, adapting themselves to the changes. In this way the menu plugin would have menu specific code for it, instead of having it in the skin files.

comment:4 Changed 16 years ago by Artur Formella

It is possible to change context menu UI color only once.

comment:5 Changed 16 years ago by Tobiasz Cudnik

@Frederico: It's not fully possible to move menu-related code to menu plugin, because CSS is skin-dependent, so it will have to stay inside skin files. Although keeping references inside menu objects and listening for an uiColorChanged event may be a good solution.

@Artur: It's a know issue (there is a ticket somewhere i think). That's because missing reference to style element at some point. It needs deeper investigation.

Changed 16 years ago by Tobiasz Cudnik

Attachment: 4048_2.patch added

comment:6 Changed 16 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Second patch changes relation with menu plugin to be event based, which resolves both submenu and sequential uiColor changes issues (mentioned by Artur). Also code is now more general purpose and can manage ui color more easily in any iframe document.

comment:7 Changed 15 years ago by Garry Yao

Keywords: Review- added; Review? removed

The patch works for me in all browsers, while there're some issues noticed in codes:

  1. L160 - L170 of skin.js could be merged into 'updateStylesheets' function;
  2. It's better to use RegExp for global replacement;
  3. L176 - L192 should be merged within the guard condition at L195.

Changed 15 years ago by Tobiasz Cudnik

Attachment: 4048_3.patch added

comment:8 in reply to:  7 Changed 15 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Replying to garry.yao:

The patch works for me in all browsers, while there're some issues noticed in codes:

  1. L160 - L170 of skin.js could be merged into 'updateStylesheets' function;
  2. It's better to use RegExp for global replacement;
  3. L176 - L192 should be merged within the guard condition at L195.

1 and 2 are fixed.

Although 3 is incorrect, because UI color functionality should be available independently from having a uiColor setting in a config. It can be set using editor.setUiColor() in any moment and we shouldn't block it.

comment:9 Changed 15 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:10 Changed 15 years ago by Tobiasz Cudnik

Resolution: fixed
Status: assignedclosed

Fixed with [4180].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy