Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4028 closed Bug (fixed)

Maximize control's tool tip is wrong once it is maximized

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

Description

Once the editor is maximized using the maximize control, the maximize control's tooltip should show as 'Minimize' instead of that it still shows as 'Maximize'

Attachments (5)

4028.patch (23.0 KB) - added by Tobiasz Cudnik 8 years ago.
4028_2.patch (23.7 KB) - added by Tobiasz Cudnik 8 years ago.
4028_3.patch (24.1 KB) - added by Tobiasz Cudnik 8 years ago.
4028_4.patch (23.8 KB) - added by Tobiasz Cudnik 8 years ago.
4028_4_postfix.patch (513 bytes) - added by Tobiasz Cudnik 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added

comment:2 Changed 8 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4028.patch added

comment:3 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added

comment:4 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Instead of augment toolbox::getItem, I think it's better if we could introduce a method for commands which retrieve the associate ui button if exist as CKEDITOR.command::getUIButton, any button related manipulation could happen based on it:

var cmdButton = this.getUIButton();

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4028_2.patch added

comment:5 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Second patch introduces CKEDITOR.command::getButtons() method, which will return all buttons related to a command.

In a way i see this is that we should keep toolbox::getItem method as it's more related to a toolbar. Besides that we should provide some more relation between toolbar markup, toolbar object and toolbar definition, which are right now not linked in any way. Also possibility of re-rendering parts of the toolbar, while still keeping existing references can be considered as useful.

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

This patch creates a dependency between the core code and the plugins. The _source/core/command.js file should not be touched for it, but it should be managed at the toolbar plugin (which could extend the command class).

Another point is that the toolbar doesn't hold buttons only. It can have any kind of UI element. So, the function could be named more generically, and considering that it's limited to the toolbar elements it could be named getToolbarItems.

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4028_3.patch added

comment:7 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

comment:8 Changed 8 years ago by Tobiasz Cudnik

With some help from Garry i'm proposing simplified patch with no additional method and command->ui references.

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4028_4.patch added

comment:9 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

Please remove the unused prototype fields 'editor' and 'uiItems' there before commit it.

comment:10 Changed 8 years ago by Tobiasz Cudnik

Resolution: fixed
Status: assignedclosed

Fixed with [4137].

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4028_4_postfix.patch added

comment:11 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review+ removed

IE6 have issues with used guard statement. I'm attaching quick fix for it.

comment:12 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:13 Changed 8 years ago by Tobiasz Cudnik

Post fixed with [4139].

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