ttkwidgets icon indicating copy to clipboard operation
ttkwidgets copied to clipboard

Merge formateli:onoffbutton into master to add OnOffButton

Open formateli opened this issue 5 years ago • 8 comments

PR Details:

  • Widget name: OnOffButton
  • Author: @formateli

Description

A simple On/Off button (aka switch button)

Checklist

  • [x] Widget in a separate file in the appropriate folder
  • [ ] Widget functions properly on both Windows and Linux
  • [x] Widget code includes docstrings with parameter descriptions
  • [x] Included an example file in /examples
  • [x] Widget is covered by unitttests in /tests
  • [ ] Widget includes required assets files
  • [ ] Reference to widget in AUTHORS.md
  • [x] Entry in sphinx documentation

formateli avatar Apr 21 '20 17:04 formateli

Hi everyone. I have developed this widget for a project and I would like to know your observations about it, in order to improve it if necessary.

formateli avatar Apr 21 '20 17:04 formateli

I'll review tomorrow in more details, but there are a few things that I see are wrong with this widget, like taking in StringVar, IntVar or BooleanVar types for the variable, typos in the docstrings, and the name of the widget (should probably be ToggleButton instead, states are not on/off all the time).

sbordeyne avatar Apr 21 '20 21:04 sbordeyne

I followed the checkbutton widget as a guidance, which is in essence a toggle button too. The other name I considered for the widget was SwitchButton

formateli avatar Apr 22 '20 00:04 formateli

Thank you for your pull request! While I quite like the idea of having a widget like this, I'm not quite happy with how this widget looks. Personally, I'd prefer to have an implementation based on new UI elements and layouts with the Checkbutton at its core but the looks of a toggle.

This would be quite a bit harder than a Canvas but it has the potential to look a lot better, possibly even themed if it could be integrated with ttkthemes. If that is something you want to pursue, then I am open to working with you to achieve it.

Right now, the widget should at least have the following:

  • Get its colours from the active ttk theme, so with ttk.Style().lookup("foreground") or something similar.
  • Antialiasing of some sort, to fix the ragged edges of the fields.
  • Support for clicking on the indicator as well as the green/red part of the widget.

Particularly the anti-aliasing is a problem, though it is probably not impossible. Still, it would be slow when using a Canvas so still not ideal. Are you open to working on this some more, @formateli ?

RedFantom avatar May 06 '20 15:05 RedFantom

Hi, I agree that the widget does not look very good, it is a prototype and has no sense continue with it. I would like to develop one like the one you mention, please point me at some examples and/or some guidence on how to implement it.

formateli avatar May 07 '20 17:05 formateli

Okay, so the way I envision it is this:

  • The ToggleButton defines custom styles much like the unmerged Notebook widget does. This is advanced Tkinter, and figuring out how it work might take you a few tries. The Notebook widget is really a solid example. These custom elements provide a baselines for when ttkthemes is not available or a theme without those styles is selected.
  • ttkthemes may then be modified to create the styles within the themes, providing themed versions of these widgets. It would basically create a wholly new themed ttk widget. This way, the widget would blend in properly with the application.

As a baseline you could use the images from a GTK theme, such as arc, though something a little more neutral may be more appropriate.

If you can do the first part, I can do the second. Currently I am working on another Tkinter project so I won't have time to build such a ToggleButton in the near future.

RedFantom avatar May 08 '20 14:05 RedFantom

Codecov Report

Merging #61 into master will increase coverage by 0.10%. The diff coverage is 92.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   89.69%   89.79%   +0.10%     
==========================================
  Files          36       37       +1     
  Lines        3725     3802      +77     
==========================================
+ Hits         3341     3414      +73     
- Misses        384      388       +4     
Impacted Files Coverage Δ
ttkwidgets/onoffbutton.py 92.00% <92.00%> (ø)
ttkwidgets/__init__.py 100.00% <100.00%> (ø)
ttkwidgets/utilities.py 100.00% <0.00%> (ø)
ttkwidgets/frames/balloon.py
ttkwidgets/frames/tooltip.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ff737ff...21c313a. Read the comment docs.

codecov-io avatar May 09 '20 21:05 codecov-io

Thanks for your guidance. Once I had a better understanding of how styles work in the ttk widgets, I just had to derive the new OnOffButton from ttk.Checkbutton and create new element and layout for this class. Awaiting your comments

formateli avatar May 09 '20 21:05 formateli