Opened 7 years ago

Closed 7 years ago

#5307 closed Bug (fixed)

Customizing multi-tab dialog to show only one tab doesn't work

Reported by: Damian Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: UI : Dialogs Version: 3.0
Keywords: IBM Confirmed Review+ Cc: Joek

Description (last modified by Garry Yao)

Take the Find dialog as an example. Customize it by hiding the Replace tab and leaving only the Replace tab visible:

replaceTab.hidden=true;
dialogDefinition.dialog.parts.dialog.addClass('cke_single_page'); 

This results in a single Find tab appearing in the dialog, but the tab can be accessed with keyboard and when navigating using arrows, an error occurs and the content of the dialog becomes blank.

Attachments (2)

5307.patch (1.3 KB) - added by Garry Yao 7 years ago.
5307_2.patch (2.2 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by Damian

I neglected to mention that I do not want to completely remove the Replace tab, i.e.

dialogDefinition.removeContents('replace');

comment:2 Changed 7 years ago by Garry Yao

Component: GeneralUI : Dialogs
Description: modified (diff)
Keywords: Confirmed added
Owner: set to Garry Yao
Status: newassigned
Version: 3.0

Changed 7 years ago by Garry Yao

Attachment: 5307.patch added

comment:3 Changed 7 years ago by Garry Yao

Keywords: Review? added

comment:4 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

I think that the changes of the patch are good, but don't fully address this problem. Even testing the about dialog http://ckeditor.com/demo it's possible to get a js error.

  1. Open the dialog
  2. Press Alt+F10 twice
  3. Press Tab -> "O is undefined" (fails in selectPage with null as the id)

So we need to avoid any try to focus the tabs if there's only one. Then this patch will help to recognize the situation for modified dialogs.

I think that the final touch for this patch would be to verify that when hidePage and showPage are called, the 'cke_single_page' class is applied automatically (and whatever else to make it behave as a multitab/no-tabs dialog). That way people can manage the tabs properly through the API without caring about this internal details.

Changed 7 years ago by Garry Yao

Attachment: 5307_2.patch added

comment:5 Changed 7 years ago by Garry Yao

Keywords: Review? added; Review- removed

With the above comments revised.

comment:6 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ added; Review? removed

comment:7 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5246].

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