Opened 5 years ago

Closed 4 years ago

#11970 closed defect (fixed)

wxDataViewChoiceRenderer set/get methods should use the current selection index not a string

Reported by: HansR Owned by:
Priority: normal Milestone: 3.0.0
Component: GUI-all Version: stable-latest
Keywords: wxDataViewCtrl API Cc:
Blocked By: Blocking:
Patch: no

Description

At present the wxDataViewChoiceRenderer's set and get methods accept and return strings, respectively. This means that any data model that uses this class has to read the strings and work backward to get the choice that the user entered. It is the choice, and not the string describing the choice, that the application needs, and the current method makes getting this information complicated.

It would be much better if the current selection index were used instead. The string could still be used internally by the renderer, but the data model needs the selection index.

Change History (10)

comment:1 Changed 5 years ago by HansR

I don't know if GUI-all is the right component, but I didn't see a component listed for wxAdvanced.

comment:2 Changed 5 years ago by vadz

  • Keywords wxDataViewCtrl API added
  • Milestone changed from 2.9.2 to 3.0
  • Status changed from new to confirmed

I agree that using indices would be better.

comment:3 Changed 4 years ago by roebling

Thanks for the suggestion. I don't think we should the existing class and we cannot make it work with both "string" and "long", so we'd need to add another class ("wxDataViewChoiceByIndexRenderer"?) that would hold an integer and which would do the translation from string to int.

comment:4 Changed 4 years ago by RR

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

(In [64327]) Added quick implementation of wxDataViewChoiceByIndexRenderer, closes #11970 (wxDataViewChoiceRenderer set/get methods should use the current selection index

comment:5 Changed 4 years ago by HansR

Just to let people know that the class is called wxDataViewChoiceRendererByIndex and not wxDataViewChoiceByIndexRenderer (in rev 64327).

comment:6 Changed 4 years ago by HansR

  • Resolution fixed deleted
  • Status changed from closed to reopened

I just tried the wxDataViewChoiceRendererByIndex, but get an exception when trying to convert the updated value from the editor control (I'm using wxAny, so I get an exception instead of a 0 value). It looks like the code from the original wxDataViewChoiceRenderer that is used when the editor control is closed is being executed, and still sending a string to the data model, and not an index.

comment:7 Changed 4 years ago by jmsalli

Hi,

If the crash happens only with wxAny and not with wxVariant, then I'd really want to take look at it. Is there a way to (easily) reproduce it in a wxDataViewCtrl sample, for instance?

Thanks!

comment:8 Changed 4 years ago by roebling

No, my code was to simple. We need to write a new renderer for each platform, since e.g. GTK+ set the data to the model in a private method (GtkOnTextEdited).

comment:9 Changed 4 years ago by HansR

The crash occurs when using wxAny because wxAny recognizes that the data isn't of type long. If you use the wxVariant directly then it doesn't crash, but the returned value is 0 every time. I think that what's missing is an implementation of:
virtual bool GetValueFromEditorCtrl (wxControl *editor, wxVariant &value);

Since wxDataViewChoiceRendererByIndex has been made a child-class of wxDataViewChoiceRenderer, if this method is missing, then it still returns a string object instead of an index.

comment:10 Changed 4 years ago by RR

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

(In [64378]) Second try to get wxDataViewChoiceByIndex and its name right, hopefully fixes #11970: wxDataViewChoiceRenderer set/get methods should use the current selection
index not a string

Note: See TracTickets for help on using tickets.