Skip to content

fix(ring_progress_bar): added hover mouse effect#1031

Open
wyzula-jan wants to merge 6 commits intomainfrom
feat/ring-progress-enhanced
Open

fix(ring_progress_bar): added hover mouse effect#1031
wyzula-jan wants to merge 6 commits intomainfrom
feat/ring-progress-enhanced

Conversation

@wyzula-jan
Copy link
Contributor

@wyzula-jan wyzula-jan commented Jan 28, 2026

Description

Hover widget extension, ring minor improvements, hover tooltip with additional info over ring.

Type of Change

  • UI/UX improvement for RingProgressBar hover interactions and tooltip presentation
  • Bug fix in ring settings handling for device/signal selection without requiring the container to be a BECWidget

How to test

  • Open a RingProgressBar widget with multiple rings with different modes and move the mouse over each ring
  • Launch scan and hover over the rings, ring should be highlighted and hover should appear with additional data.
  • check that hover data are up to date and updates with the scan progress

Screenshots / GIFs (if applicable)

ring_compressed.mp4

Additional Comments

The tooltip implementation was refactored to use the shared WidgetTooltip from hover_widget.py, which is used for example in the BECMainWindow scan progress bar in the bottom right.

Copilot AI review requested due to automatic review settings January 28, 2026 09:52
@wyzula-jan wyzula-jan self-assigned this Jan 28, 2026
@wyzula-jan wyzula-jan force-pushed the feat/ring-progress-enhanced branch from 8f1e948 to 92713a1 Compare January 28, 2026 09:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds hover effects to the ring progress bar widget, including visual animations (line width and radius expansion) and informative tooltips that display ring mode, values, and device information when hovering over individual rings.

Changes:

  • Added mouse tracking and hover detection for individual rings within the progress bar container
  • Implemented smooth hover animations with line width and radius expansion effects
  • Added dynamic tooltips showing ring configuration details (mode, value, max_value, and device/signal information for device mode)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File Description
bec_widgets/widgets/progress/ring_progress_bar/ring_progress_bar.py Added mouse event handlers, ring detection logic at cursor position, hover state management, and tooltip building functionality
bec_widgets/widgets/progress/ring_progress_bar/ring.py Implemented hover animation with QPropertyAnimation, modified paint event to render hover effects (line width and radius expansion), and added hover_progress property

Note: The PR description references issue #123 which concerns stuck communication with BECFigure during automatic updates, which is unrelated to this hover functionality. Additionally, the PR title uses "fix" prefix, but this appears to be a new feature addition rather than a bug fix.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Jan 28, 2026

@wyzula-jan wyzula-jan marked this pull request as draft January 28, 2026 13:39
@wyzula-jan wyzula-jan changed the base branch from pre_release_v3 to main March 6, 2026 09:20
@wyzula-jan wyzula-jan force-pushed the feat/ring-progress-enhanced branch 2 times, most recently from 6028210 to a02170c Compare March 16, 2026 15:13
Copy link
Contributor

Copilot AI commented Mar 17, 2026

@wyzula-jan I've opened a new pull request, #1101, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

bec_widgets/widgets/progress/ring_progress_bar/ring_progress_bar.py:374

  • clear_all() closes and deleteLater()s rings but does not call ring.cleanup() (unlike remove_ring). If a ring was connected to scan/device endpoints, this can leave dispatcher subscriptions around until the ring object is GC’d, with a risk of callbacks hitting a partially-deleted widget. Call ring.cleanup() (and ideally stop hover animation) before close()/deleteLater().
        self._hovered_ring = None
        self._last_hover_global_pos = None
        self._hover_tooltip.hide()
        for ring in self.rings:
            ring.close()
            ring.deleteLater()
        self.rings = []

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

Copilot AI commented Mar 17, 2026

@wyzula-jan I've opened a new pull request, #1102, to work on those changes. Once the pull request is ready, I'll request review from you.

@wyzula-jan wyzula-jan force-pushed the feat/ring-progress-enhanced branch 2 times, most recently from 8e2705f to 4338bb7 Compare March 17, 2026 13:31
@wyzula-jan wyzula-jan force-pushed the feat/ring-progress-enhanced branch from 4338bb7 to 62fde02 Compare March 17, 2026 14:02
@wyzula-jan wyzula-jan marked this pull request as ready for review March 17, 2026 14:12
@wyzula-jan wyzula-jan enabled auto-merge (rebase) March 17, 2026 14:12
@wyzula-jan wyzula-jan requested a review from a team March 17, 2026 14:12
Copy link
Member

@wakonig wakonig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the placement of _refresh_hover_tooltip. Some update methods have it, some don't, some have it even 3 times (cf. set_update)

if refresh_tooltip:
if self.progress_container and self.progress_container.is_ring_hovered(self):
self.progress_container.refresh_hover_tooltip(self)
super().update()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just self.update()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants