#235 TSplitButton - encapsulation of Split Button

[Ognyan Chernokozhev]

A new button type introduced with Windows Vista is the Split button, which uses the style BS_SPLITBUTTON or BS_DEFSPLITBUTTON.

We should make a new class that derives from TButton, and adds support for the Split button specific functionality like the BCM_GETSPLITINFO, BCM_SETDROPDOWNSTATE and BCM_SETSPLITINFO messages and the BCN_DROPDOWN notification.

Additionally, since a common use case of the Split button is to display a menu under the button as a response to the clicking the dropdown arrow, we should provide a simplified interface, in which a menu can be assigned to the control, and it will internally manage it and display it in the correct place.

See discussion https://sourceforge.net/p/owlnext/discussion/97177/thread/5acc6bb075/

[Ognyan Chernokozhev]

Initial work in [r6456].

Still need to design the interface for setting a dropdown menu and for getting and setting the drop down image list.

[Vidar Hasfjord]

Hi Ognyan,

I've reviewed your implementation [r6456] and [r6459]. Very nice! I like the clean modern C++ code and simple interface.

Issues:

• AssignDropDownMenu and BcnDropDown should be documented.
• BcnDropDown should not succeed if GetParent returns nullptr. Since a missing parent is a program logic error that should never occur in a release build, I suggest we add a PRECONDITION and then remove the test in the if-statement.
• Pedantically, BcnDropDown should call DefaultProcessing if no menu is assigned (even though DefaultProcessing apparently doesn't do anything).
• BcnDropDown can be simplified. The client rectangle of the button is a member of the function parameter and can be converted to screen coordinates by calling TButton::MapClientToScreen without the use of the parent.
• SetDropDownState should use a more descriptive parameter name.
• Header should include "memory" (for std::unique_ptr).
• In the implementation file, there is a global using-directive for namespace std, so "std::" qualification in the source can be removed. Alternatively, remove the using-directive.
  
  

Except for documentation, these issues have been addressed in [r6460].

Edit: OWL5_COMPAT mode regression was fixed in [r6461].

[Ognyan Chernokozhev]

Hi, Vidar,
Thanks for the code review and the fixes.

Regarding [r6461], I wonder - do we still need to maintain OWL5_compat in OWLNext 9.x?
(especially considering we do not have anything like OWL6_COMPAT or OWL7_COMPAT), and there may be quite some 6.x code that would not compile without modifications with latest versions)

If someone still needs to upgrade from OWL, the recommended way is to use at least 6.44 as an intermediate step, and that should include removal of any OWL5_COMPAT code.

[Vidar Hasfjord]

do we still need to maintain OWL5_COMPAT [on the trunk]?

If this is your way of asking to end support for it, I couldn't be happier. :-)

I think your arguments make sense. OWL5_COMPAT mode is probably of little use at this point. By now, the deprecation features in modern C++ (and in Doxygen) are probably sufficient tools to retain compatibility while making API changes.

Removing OWL5_COMPAT would clean up the code and allow more modern C++ to be used (such as macro-free response tables). It would also enable more (nice) things from Owlet to migrate to the trunk as well. For example, the Windows message dispatch is neater in Owlet, which is something I have written about in the past [news:2019/11/5-year-review-of-the-640-series].

So, my vote is for the removal of OWL5_COMPAT.

[Ognyan Chernokozhev]

Yes, I support removing the removal of OWL5_COMPAT from OWLNext 8.x.

I admit that for quite some time, when I build the libraries from trunk, I don't check the OWL5_COMPAT option, so that the build finishes quickly.

[Vidar Hasfjord]

@jogybl wrote:

I support removing the removal of OWL5_COMPAT from OWLNext 8.x.

Good! Do you want to solicit feedback from the wider community on the issue, or do you think it is appropriate to make an executive decision, considering the lack of broader participation on the trunk? In the latter case, go ahead!

Tip: After removing OWL5_COMPAT mode it would be helpful to have a check in the source code that produces an error message if the macro is defined.

Regarding necessary changes in OWLMaker, I have them ready to go. The changes needed comprise just a couple of simple overrides of TToolset::SupportsCompatibilityMode and an update of the description of the OWLNext API compatibility mode on the libraries page. Your OWLMaker design is holding up very well!

I admit that for quite some time, when I build the libraries from trunk, I don't check the OWL5_COMPAT option, so that the build finishes quickly.

I have a 16-core AMD Ryzen 5950X in my desktop computer now, so full builds are less of a pain. Still, I don't routinely build the dynamic link option. I only include that mode for full build testing before committing substantial changes.

PS. After removing OWL5_COMPAT, you can of course revert [r6461].