#815 wxSmith: Default Color is always Black

[elvissteinjr]

I'm using SVN Build 11628. Not sure when it started happening, but I could also observe in the previous nightly.

Basically, selecting "Default" as color in wxSmith results in a custom color setting with black (0, 0, 0) as color instead. Existing color is fine when already set to "Default" as long as the setting isn't touched.

[Teodor Petrov]

OS? We have all night builds on the download server it would help a lot if you can try to find the night build which introduced this.

Does it happen with a simple project created from scratch?

[bluehazzard]

This was probably with the switch to wx3.XX
I work on this and as far as i can tell, if you set the color value to something and then back to default wxSmith trades the default color as custom color.

I think this has something to do with wxwidgets\properties\wxscolourproperty.cpp:

#if wxCHECK_VERSION(3, 0, 0)
    wxString wxsMyColourPropertyClass::ValueToString( wxVariant& value,
                                                    int argFlags ) const
    {
        wxColourPropertyValue val = GetVal(&value);

        int index;

        if ( argFlags & wxPG_VALUE_IS_CURRENT )
        {
            // GetIndex() only works reliably if wxPG_VALUE_IS_CURRENT flag is set,
            // but we should use it whenever possible.
            index = GetIndex();

            // If custom colour was selected, use invalid index, so that
            // ColourToString() will return properly formatted colour text.
            if ( index == GetCustomColourIndex() )
                index = wxNOT_FOUND;
        }
        else
        {
            index = m_choices.Index(val.m_type);
        }

        return ColourToString(val.m_colour, index, argFlags);
    }
#else
    wxString wxsMyColourPropertyClass::GetValueAsString(cb_unused int argFlags) const
    {
        wxColourPropertyValue val = GetVal();

        int ind = GetIndex();

        // Always show custom colour for textctrl-editor
        if (val.m_type == wxPG_COLOUR_CUSTOM)
        {
            ind = wxNOT_FOUND;
        }

        if (val.m_type == wxsCOLOUR_DEFAULT)
        {
            return wxsColourLabels[ind];
        }

        return ColourToString(val.m_colour, ind);
    }
#endif

the wx30 code does not handle the wxsCOLOUR_DEFAULT case separately... Still investigating...

[bluehazzard]

I am not 100% sure what all this code does... But after some debugging and reading and trying i think this is the right fix:

diff --git a/src/plugins/contrib/wxSmith/wxwidgets/properties/wxscolourproperty.cpp b/src/plugins/contrib/wxSmith/wxwidgets/properties/wxscolourproperty.cpp
index 6fda231c2..4d8d5bca2 100644
--- a/src/plugins/contrib/wxSmith/wxwidgets/properties/wxscolourproperty.cpp
+++ b/src/plugins/contrib/wxSmith/wxwidgets/properties/wxscolourproperty.cpp
@@ -416,11 +416,15 @@ namespace
         {
             ind = GetIndexForValue(cpv.m_type);
         }

-        else
+        else if (cpv.m_type == wxPG_COLOUR_CUSTOM)
         {
-            cpv.m_type = wxPG_COLOUR_CUSTOM;
             ind = GetCustomColourIndex();
         }
+        else
+        {
+            cpv.m_type = wxsCOLOUR_DEFAULT;
+            ind = 0;
+        }
     }
     else
     {

It also makes the code symmetric with the wx28 code. Jens ported this code, so it would be nice if he could confirm my change....

[Teodor Petrov]

You have to ping in with a PM in the forum. I'm not sure it follows the bug tracker these days.

[Jens Lody]

To be honest, I do not recall why I did it that way, but it's about seven years ago.
It looks like I totally missed the default-colour.
@bluehazzard: your proposed fix seems to be correct, if it works, just go ahead.

[bluehazzard]

Hopefully fixed in revision: 11687

Thank you all for your comments and for the report of this ticket