feat: toggle switch

[hanjinliu]

I have been using toggle switch in many places of my repositories and I thought that it would be better to have one in superqt.

[codecov]

Codecov Report

Attention: Patch coverage is 94.82759% with 9 lines in your changes missing coverage. Please review.

Project coverage is 87.00%. Comparing base (22372f5) to head (6cea5e1).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/superqt/switch/_toggle_switch.py	94.73%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
- Coverage   87.28%   87.00%   -0.28%     
==========================================
  Files          47       49       +2     
  Lines        3554     3772     +218     
==========================================
+ Hits         3102     3282     +180     
- Misses        452      490      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Czaki]

why is there a need to have two separate class? Why could it not be as QCheckBox that could be labeled by additional constructor argument or call the method?

Why does it not inherit from QCheckBox?

[hanjinliu]

Hi @Czaki , thank you for your feedback.

Why could it not be as QCheckBox that could be labeled by additional constructor argument or call the mathod?

You are right... 🥲

Why does it not inherit from QCheckBox?

Is there any benefit inheriting QCheckBox?

[tlambert03]

thanks @hanjinliu! this is awesome. and something i've wanted as well.

I agree that a single class is probably sufficient.

Since this is most like a QCheckBox, making it a drop-in replacement for QCheckbox would be great (perhaps you already did that? I haven't looked closely yet... but the methods should be the same when possible). Inheriting from QCheckBox would be nice for isinstance checks, and findChildren type searches. But let us know if you feel like you have to bend over backwards to make that happen (in which case it might not be worth it)

[Czaki]

Is there any benefit inheriting QCheckBox?

That it could be used as a checkbox replacement. It will make mypy happy to work with function that accept already QCheckBox. It will be easier to use in code for autogenerate GUI (for example, magicgui may in some moments allow configuring toggle switch in place of check box) etc

[hanjinliu]

hmm, something went wrong in old pyqt6 ... but probably we can just skip that test.

[tlambert03]

Thanks for your work on this @hanjinliu, I had a moment to take a bit of a closer look.

I think there are many things that could potentially be improved with regards to making this feel more integrated with the styling of other widgets (things like awareness of widget active state and possibly hover state). and there's a couple small bugs at the moment:

• unlike for QCheckBox: checkStateChanged gets emitted when you click on the button, but not when you click on the text (even though the check state does indeed change).
• the sizeHint doesn't quite seem to work for width. if you enter a label with a long word QToggleSwitch("SomeLongWord"), it gets cropped on the right
• All calls to fixed sizes need to be removed like (setFixedSize and setMaximumHeight, since those always end up playing poorly eventually (even if they initially solve a style problem). I think this means you'll need to add more logic to determine the exact rect for the rounded rect (rather than just assuming you can fill the entire widget width/height).
• def size(self) -> int: doesn't match the parent API. size() needs to return a QSize... so can't be used for as a synonym for toggle switch width.
• in looking closer, I think we should subclass QAbstractButton, rather than QCheckBox, but implement as many methods as possible to mimic the final QCheckBox API where appropriate (leaving out things like tristate compatibility)
• Since QAbstractButton already has text()/setText(), I think you should use that rather than creating a new QLabel widget (you will need to add it to your paintEvent). This means the whole thing will be one QAbstractButton rather than something containing a _QToggleSwitch inner widget. (this avoids a couple edge cases with composing widgets too)

When implementing paintEvents from scratch I think looking through qtbase is very helpful for learning patterns. for checkbox it looks like this:

first initialize a style option that contains all the instructions for the styles (this is a nice pattern that first puts all of the style related things into a single object)
https://github.com/qt/qtbase/blob/3c919b6d0df486b7319faf8e4f366ab457e9f852/src/widgets/widgets/qcheckbox.cpp#L286-L292

use that in the paint event, calling lower level primitives to draw grooves, controls, etc...
https://github.com/qt/qtbase/blob/3c919b6d0df486b7319faf8e4f366ab457e9f852/src/widgets/styles/qcommonstyle.cpp#L1410-L1429

but, to be clear: I don't expect you to necessarily reconsider the entire design following the C-style patterns just for this PR. I really appreciate having something rather than nothing, and what you have here is an excellent start. I would, however, like to leave open the possibility of adding even more low level native integrations in the future, without breaking things.

maybe just address any of the points above that you think are important before merge and ping me when ready?

[hanjinliu]

Thank you for your review @tlambert03 !
Yeah, I agree that QAbstractButton is a good abstraction to keep the API concise and meaningful.
At this moment, I think the important things that must be addressed in this PR are to inherit QAbstractButton and properly implement paintEvent for better customizability, as these are not easy to be fixed in the future without breaking API changes. For the widget size styling, I'm not sure how painful it would be to improve this, but let me try if I can solve.
I think I will address these issues this weekend and let you know when things are ready.

[hanjinliu]

Hey @tlambert03 , I think problems are solved.
Widget styling with a style option object is actually very reasonable. Could you please check if the widget looks nicely?

I also added drawGroove() and drawHandle(), which I think will help users easily customize the widget design (Example code is in examples/toggle_switch.py).

[tlambert03]

this is fantastic work @hanjinliu ... love what you did.

I've made some comments and suggestions in a PR to your branch hanjinliu#1
mostly related to using as much of the superclass functionality as possible, and using the style option when possible for querying state during drawing

[Czaki]

This PR looks really nice. Next to @tlambert03 remarks I have few small.

[Czaki]

Suggested change

	def test_signal_emission_order(qtbot):
	def test_signal_emission_order(qtbot):
	"""Check if event emmision is same for QToggleSwitch and QCheckBox"""

[Czaki]

Hm. Is there a way to integrate it with qss styles (like for napari). Especially for toggling theme?

[tlambert03]

He made them Qproperties… which I believe means you can style with qss

[tlambert03]

Yep: https://doc.qt.io/qt-6/stylesheet-syntax.html#setting-qobject-properties

[hanjinliu]

Thank you for your thorough review @tlambert03 ! I was wondering why my widget didn't work with the super-class methods but setCheckable(True) was everything I needed 🤣
Maybe it's better to add an example of using stylesheet? I'll make some minor updates and push later.