wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
6.12k stars 1.77k forks source link

Refactor wxTextEntryBase and extract wxTextEntryIface from it #18071

Open wxtrac opened 6 years ago

wxtrac commented 6 years ago

Issue migrated from trac ticket # 18071

component: GUI-all | priority: normal | keywords: wxTextEntry

2018-01-29 19:30:58: @vadz created the issue


Current wxTextEntryBase is a mess because it's used both as an abstract interface and as a concrete base class implementing some of its functionality, e.g. hints support.

This results in problems such as #16998, where a derived class overrides some methods forwarding them to another wxTextCtrl, but not all of them, breaking the assumptions in the base class implementation.

We need to split wxTextEntryBase in a purely abstract wxTextEntryIface, defining the interface, while keeping the default implementation in wxTextEntryBase itself and inherit classes such as wxComboBox and wxSearchCtrl from wxTextEntryIface only (and not wxTextCtrlIface, as it does now, but which makes no sense because it can't really implement any of text area methods).

For convenience we should also provide wxTextEntryDelegate similar to e.g. wxDelegateRendererNative, which would implement wxTextEntryIface by simply delegating all of its methods to another object implementing the same interface (e.g. a wxTextCtrl instance).

To anybody interested in a real-life C++ refactoring exercise: please feel free to try doing this, it's not really urgent, so I don't think I'm going to have time to do it until 3.2.0, but it would be definitely nice to fix this mess.

wxtrac commented 6 years ago

2018-01-30 14:26:07: @vadz commented


In 58ac3d3690f05c98c2a377a7fe92c14ed11e9ca8: Fix wxSearchCtrl::ChangeValue() to actually change value

This was broken because wxSearchCtrl inherited the base class version of ChangeValue() which didn't really work for it due to the poor way in which wxTextEntry is designed (see #18071).

Closes #16998.