PDA

View Full Version : Some probable ClanGUI bugs...



user
01-04-2011, 12:33 PM
Hi, I am a ClanLib user, and I found some bugs in ClanGUI 2.2.5 system and have some suggestions about some GUI controls APIs.

1. popupmenu.cpp


CL_PopupMenu CL_PopupMenu::create_null_object()
{
CL_PopupMenu pmenu;
pmenu.impl = CL_SharedPtr<CL_PopupMenu_Impl>();
return pmenu;
}


Don't know, what's the purpose of this method. It creates an invalid popup menu, because it will have null implementation (pmenu.impl is initialized with null pointer). Probably, there's no need for such method, and CL_PopupMenu should have an API to clear the popup menu. I suggest, you should add a method to CP_PopupMenu like this:


void CL_PopupMenu::clear()
{
impl->items.clear();
impl->next_id = 0;
}


2. popupmenu.cpp again


void CL_PopupMenu::start(CL_GUIComponent *parent, const CL_Point &pos)
{
if (get_item_count() == 0)
return;

CL_MenuModalLoop *menu_ptr = new CL_MenuModalLoop(parent->get_gui_manager());
menu_ptr->start(parent, *this, pos);
}


Here the new CL_MenuModalLoop object is created, and after that it is never deleted, I checked. You should use smartpointers here. Add a smartpointer to implementation class (popupmenu_impl.h) and then use it like:


void CL_PopupMenu::start(CL_GUIComponent *parent, const CL_Point &pos)
{
if (get_item_count() == 0)
return;

if(!impl.get()->menu_ptr.get())
impl.get()->menu_ptr.reset(new CL_MenuModalLoop(parent->get_gui_manager()));

impl.get()->menu_ptr.get()->start(parent, *this, pos);
}

Worked perfectly for me.

3. checkbox.cpp
When you disable checkbox, it becomes unchecked, even it was checked before. I fixed it for me in such way, but probably there's a better way:


//changed function prototype inside of CL_CheckBox_Impl class
void create_parts(bool checked = false);



void CL_CheckBox_Impl::create_parts(bool checked)
{
part_component = CL_GUIThemePart(checkbox);
part_checker = CL_GUIThemePart(checkbox, CssStr::CheckBox::part_checker);
part_focus = CL_GUIThemePart(checkbox, CssStr::CheckBox::part_focus);

part_component.set_state(CssStr::hot, false);
part_component.set_state(CssStr::normal, checkbox->is_enabled());
part_component.set_state(CssStr::disabled, !checkbox->is_enabled());

part_checker.set_state(CssStr::normal, checkbox->is_enabled());
part_checker.set_state(CssStr::pressed, false);
part_checker.set_state(CssStr::indeterminated, checkbox->is_indeterminated());
part_checker.set_state(CssStr::checked, checked);
part_checker.set_state(CssStr::unchecked, !checked);
part_checker.set_state(CssStr::disabled, !checkbox->is_enabled());
}




void CL_CheckBox_Impl::on_enablemode_changed()
{
create_parts(checkbox->is_checked());
checkbox->request_repaint();
}

I think, there's a similar bug with indeterminated checkbox state...

4. progressbar.cpp
There's no bounding box around progress bar, so it's quite difficult to understand where is its beginning and ending. I fixed this for me adding a property to css and using this property in source code.

5. filedialog_impl.h
There is a bug in this line:


filenames.push_back(filename_buffer);

This line works incorrectly for languages different to English. Use this:


filenames.push_back(CL_StringHelp::ucs2_to_utf8(fi lename_buffer));


Also, multiselect will not work, because lines which implement it are commented.

6. CL_ComboBox cannot be disabled for now, but I think you know about it. I implemented this functionality for me without changing the display style, just disabling message handlers.

7. CL_ListView should have some method like scroll_to_bottom(). I use this component as a control to display the program log, so I implemented this functionality combining set_position for CL_ListView scrollbar and CL_ListView_Impl::on_scroll().

That's all for this time, sorry if I posted some duplicates.

Magnus Norddahl
01-04-2011, 07:58 PM
A lot of interesting changes. Thanks a lot for taking your time contributing back to the project.

But is it possible for you to post a subversion patch file (see the PATCHES file in clanlib for details) with your changes? It makes it a lot easier for us to see exactly what you changed and also apply such changes if we agree with them. Each of your before+after sections can be viewed in the actual source code then, which helps a lot when determining what the problem is and how your change fixes it.

user
01-05-2011, 01:12 PM
I created the patch-file, it includes:

Added clear() method to CL_PopupMenu
Fixed memory leakage in CL_PopupMenu (start() method)
Fixed bug: When you disable checkbox, it becomes unchecked, even it was checked before
Added progress bar bounding rectangles properties to themes CSS
Fixed file dialogs controls, now they're working correctly with any language
CL_ComboBox can be disabled now, but its button's visual style is not changing for now
Added get_scroll_position(), get_scroll_max_position(), set_scroll_position() methods to CL_ListView
Implemented CL_LineEdit_Impl::input_mask_accepts_input() method
Fixed CL_LineEdit bug: if numeric mode is on, you can press Ctrl+V and paste anything to textbox, even non-numerical characters

user
01-08-2011, 06:41 PM
Up...
-----

rombust
01-08-2011, 07:51 PM
Looks perfect to me.

Since the existing API is not changed (only enhanced) it can be applied to both ClanLib 2.2 and 2.3 SVN

(I can do that next week)

rombust
01-10-2011, 01:32 PM
Patch applied.

Many thanks

(Will be in the 2.2.6 release)