-
Notifications
You must be signed in to change notification settings - Fork 484
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
Horizontal Scroll Bar Span Graph Timeline #2421
base: main
Are you sure you want to change the base?
Horizontal Scroll Bar Span Graph Timeline #2421
Conversation
- Adding Horizonal Scrollbar on timeline to give more user functionality on scrubbing spans - Adding Tests for new functionality to check that it scrubs and that it is hidden when there are no anchor Signed-off-by: Cory Barney <[email protected]>
Got better direction on requirements making some changes |
- Refactoring scrollbar to be top 40% of the visable anchor area of a selected time range Signed-off-by: Cory Barney <[email protected]>
maybe a small bug - when there is a selection, the Hand icon for dragging only shows in the top part of the selection (correctly), but dragging also works if started outside of that selection in the top part (where hand icon is not shown). I think it's better to limit dragging to work as moving only within the selection. |
Will do, thanks for pointing that out! |
I think this is more in line with what your goal was, as well I noticed that the vertical red bar is present on the any place you can start dragging to create anchor points to I put that in. As well I synced up the cursor icon between the ViewingLayer and the TimelineViewingLayer so that they are the crosshair but I can swap that out to be whatever icon if you think the cross is no good. Let me know and ill probably cut this into it's own PR as it kind of when on a tangent and don't want to overload it Bug Detected: the redbar pops up when dragging but the cursor is present in the bottom 60% of the View 5ed169d_._api_v1_fail.simple_http_sentence.Jaeger.UI.-.Brave.2024-08-30.10-41-18.mp4 |
0a32ebc_._dispatch.frontend.Jaeger.UI.-.Brave.2024-08-30.12-14-50.mp4Thats is the current kick so let me know if that is perfect and ill code up the tests for it |
still seeing this bug #2421 (comment) |
0a32ebc_._dispatch.frontend.Jaeger.UI.-.Google.Chrome.2024-08-30.21-05-31.mp4I tried to replicate the functionality as close as possible to the google inspector and it has it global kind of like an invisible scroll bar. I can limit its functionality to just the physical range but I feel like being able to move and use your scroll wheel is a better UX. |
ok, it's fine to do like Chrome does, but I see that in Chrome the drag indeed still works both over and outside of the current selection, but it also shows the hand icon in both cases (in the top part). |
Currently it is setup to:
Should I get a better recorder as it looks fine to me in first person without the weird none precision clicks that get shown in the recording? I'm grasping at straws what the bug is as the open hand icon is just present within the 40% and dragging closed hand is present to show you are still clicking regardless of where in the screen. I can try to cut it off for the upper section but I think that is a regression. Am I misunderstanding something or having a difference in opinion (I do see the moment the crosshair comes up and will try to get the pixel sorted out) 08-31-2024_jaeger_pr_review.webm |
so what I meant was:
It would be great to have the cursor to be consistently crosshair / hand when there is a selection (and just crosshair when there is no current selection). |
- Refactoring to be an invisible scrubber part of the timeline viewer Signed-off-by: Cory Barney <[email protected]>
i can swap on property on the css and swap what ever area to what needs be. I put everything back to default cursor and have the to range in the middle of the anchors a grab icon cause we need to let the users know that there is that functionality but I can remove it. 06adc24_._dispatch.frontend.Jaeger.UI.-.Brave.2024-09-01.14-15-59.mp4 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2421 +/- ##
==========================================
- Coverage 96.61% 95.85% -0.77%
==========================================
Files 254 254
Lines 7679 7765 +86
Branches 1997 2002 +5
==========================================
+ Hits 7419 7443 +24
- Misses 260 322 +62 ☔ View full report in Codecov by Sentry. |
Which problem is this PR solving?
Description of the changes
How was this change tested?
Jaeger.UI.-.Brave.2024-08-29.15-46-14.mp4
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test