Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix a number of issues with Labeled and Range Sliders, add LabelsOnHandle mode. #242

Merged
merged 8 commits into from
May 6, 2024

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented May 3, 2024

This PR fixes a variety of issues I've encountered, mostly with labeled sliders:

  • removes calls to QApplication.processEvents() in the labeled sliders. When using this slider in one of my applications, I was observing some very strange behavior. Gradually, by removing things piece by piece, I was able to narrow it down to these calls to processEvents() in the slider. The calls are there because, without them, if you interactively do something like .setEdgeLabelMode(wdg.EdgeLabelMode.NoLabel), it will leave the handle labels in a slightly odd position, until the next time the widget is updated (which is likely to be very soon, or at least the next time the slider is touched/moved). I will try to figure out how to fix that moment of unstyled view, but the consequences of calling processEvents are probably even worse, and much harder to debug (since it can have a global effect, but no one would know where it's coming from).
  • fixes behavior of using the mouse wheel to move a rangeSlider when the two handles are very close to each other (before, this, it would first ensure that the two values were at least 3 apart)
  • fixes a ZeroDevisionError that can occasionally happen if the min and max value of a slider are the same thing
  • fixes a couple issues with the signals on QLabeledSlider not emitting (i.e. it uses the pattern already being used by the DoubleSlider)
  • adds a new LabelPosition.LabelsOnHandle enum that results in the labels being placed directly ON a slider handle. It's not always useful, but it can save space if you're also using a custom stylesheet.
  • catches an occasional RuntimeError when emitting the finished signal from ensure_main_thread
  • fixes the ability to set bar color using QSS on QLabeledRangeSliders
  • fixes the slight offset now seen on macos tick sliders

@tlambert03 tlambert03 added the bug Something isn't working label May 3, 2024
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 12 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@12f10be). Click here to learn what that means.

Files Patch % Lines
src/superqt/sliders/_labeled.py 71.05% 11 Missing ⚠️
src/superqt/sliders/_generic_slider.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #242   +/-   ##
=======================================
  Coverage        ?   87.08%           
=======================================
  Files           ?       46           
  Lines           ?     3398           
  Branches        ?        0           
=======================================
  Hits            ?     2959           
  Misses          ?      439           
  Partials        ?        0           

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

@tlambert03 tlambert03 changed the title fix: remove processEvents calls from QLabeledSliders fix: fix a number of issues with Labeled and Range Sliders, add LabelsOnHandle mode. May 6, 2024
@tlambert03 tlambert03 merged commit ba495a5 into pyapp-kit:main May 6, 2024
48 checks passed
@tlambert03 tlambert03 deleted the fix-labeled-slider branch May 6, 2024 21:43
@tlambert03 tlambert03 added the enhancement New feature or request label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

1 participant