Ticket #4028 (closed Bug: fixed)

Opened 5 years ago

Last modified 4 years ago

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

4028.patch (23.0 KB) - added by tobiasz.cudnik 5 years ago.
4028_2.patch (23.7 KB) - added by tobiasz.cudnik 5 years ago.
4028_3.patch (24.1 KB) - added by tobiasz.cudnik 5 years ago.
4028_4.patch (23.8 KB) - added by tobiasz.cudnik 5 years ago.
4028_4_postfix.patch (513 bytes) - added by tobiasz.cudnik 5 years ago.

Change History

comment:1 Changed 5 years ago by fredck

  • Keywords Confirmed added

comment:2 Changed 5 years ago by tobiasz.cudnik

  • Owner set to tobiasz.cudnik
  • Status changed from new to assigned

Changed 5 years ago by tobiasz.cudnik

comment:3 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added

comment:4 Changed 5 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 5 years ago by tobiasz.cudnik

comment:5 Changed 5 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 5 years ago by fredck

  • 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 5 years ago by tobiasz.cudnik

comment:7 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added; Review- removed

comment:8 Changed 5 years ago by tobiasz.cudnik

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

Changed 5 years ago by tobiasz.cudnik

comment:9 Changed 5 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 5 years ago by tobiasz.cudnik

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

Fixed with [4137].

Changed 5 years ago by tobiasz.cudnik

comment:11 Changed 5 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 5 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:13 Changed 5 years ago by tobiasz.cudnik

Post fixed with [4139].

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