fix: transaction details tooltip (#6292)#6329
fix: transaction details tooltip (#6292)#6329OscarG673 wants to merge 3 commits intomempool:masterfrom
Conversation
|
Thanks for your contribution to The Mempool Open Source Project. We require all contributors to agree to our Contributor License Agreement and don't seem to have one for you on file. Please see the instructions in CONTRIBUTING.md to proceed with this pull request. |
There was a problem hiding this comment.
Pull request overview
Fixes tooltip “jumping” in the block overview transaction details by separating cursor-following updates from a one-time post-layout size-aware position adjustment.
Changes:
- Refactors tooltip positioning into a reusable
updatePosition()method. - Adds a
requestAnimationFrame-basedschedulePositionAfterLayout()to update position once after the tooltip’s content/layout changes.
| schedulePositionAfterLayout(): void { | ||
| if (this.positionRafId != null) cancelAnimationFrame(this.positionRafId); | ||
| this.positionRafId = requestAnimationFrame(() => { | ||
| this.positionRafId = null; | ||
| if (this.tx && this.cursorPosition && this.tooltipElement?.nativeElement?.offsetParent) { | ||
| this.updatePosition(); | ||
| this.cd.markForCheck(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
schedulePositionAfterLayout() schedules a requestAnimationFrame callback that can still fire after the component is destroyed (e.g., tooltip disappears on hover-out), which may call markForCheck() on a destroyed view. Implement OnDestroy and cancel any pending RAF id (and null it) in ngOnDestroy() to avoid leaks and potential change-detection errors.
frontend/src/app/components/block-overview-tooltip/block-overview-tooltip.component.ts
Show resolved
Hide resolved
|
this seems to improve the jumping a bit, but introduces a lateral shaking effect instead: Screen.Recording.2026-02-19.at.6.20.34.AM.mov |
thanks for the feedback @mononaut. new.mov |
mononaut
left a comment
There was a problem hiding this comment.
much smoother!
just a few minor issues to address
| this.pendingPositionRecalc = false; | ||
| requestAnimationFrame(() => { | ||
| this.updateTooltipPosition(this.cursorPosition, true); | ||
| this.appRef.tick(); |
There was a problem hiding this comment.
this.appRef.tick() is overkill here, this.cd.detectChanges(); should be sufficient
| this.appRef.tick(); | |
| this.cd.detectChanges(); |
| } | ||
|
|
||
|
|
||
| ngAfterViewChecked(): void { |
There was a problem hiding this comment.
likewise, running these checks in ngAfterViewChecked() is excessive (it'll run on every change detection cycle across the whole application).
it should be fine to make this a simple class method and invoke it explicitly from inside ngOnChanges (when we know something relevant has actually changed)
and then you probably don't need the pendingPositionRecalc field either.
| ngAfterViewChecked(): void { | |
| private schedulePositionRecalc(): void { |
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
there are a lot of stray newlines everywhere, please remove
| } else { | ||
| y = maxYInParent; | ||
| } | ||
| y = Math.min(y, maxYInParent); |
There was a problem hiding this comment.
this seems redundant? I don't think it's possible for y to be greater than maxYInParent by this point
| y = Math.min(y, maxYInParent); |
thank you for the feedback, @mononaut. i have implemented the suggested changes in Please let me know if there's anything else to adjust! |
| private schedulePositionRecalc(): void { | ||
| requestAnimationFrame(() => { | ||
| this.updateTooltipPosition(this.cursorPosition, true); | ||
| this.appRef.tick(); |
There was a problem hiding this comment.
looks like we're still using appRef.tick() - did it not work properly with cd.detectChanges() instead?
| @@ -1,4 +1,4 @@ | |||
| import { Component, ElementRef, ViewChild, Input, OnChanges, ChangeDetectionStrategy, ChangeDetectorRef } from '@angular/core'; | |||
| import { Component, ElementRef, ViewChild, Input, OnChanges, AfterViewChecked, ChangeDetectorRef, ApplicationRef } from '@angular/core'; | |||
There was a problem hiding this comment.
AfterViewChecked is no longer used
Summary
the transaction details tooltip in the block overview sometimes jumped when moving the mouse. This happened mainly when the tooltip had more properties, meaning when the content was taller or wider
Results
the tooltip stops jumping when it has more properties: the position is adjusted only once after the layout, using the correct size, and then it follows the cursor using the same logic,
result video:
https://github.com/user-attachments/assets/2a0f306d-1cb0-41d5-a5ed-c0f037042e0f
fixes: #6292