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

Set default spanIndex based on spanId specified in URL hash #3184

Closed
wants to merge 1 commit into from

Conversation

mrajah-twttr
Copy link

@mrajah-twttr mrajah-twttr commented Aug 21, 2020

Set default spanIndex based on spanId specified in URL hash, this should scroll to the span, open up the span and highlight it

Need some guidance on how to make it scroll. the change currently works to highlight and open the details of the appropriate span @jorgheymans

…uld scroll to the span, open up the span and highlight it
@jorgheymans
Copy link
Contributor

jorgheymans commented Aug 26, 2020

Thanks, i have requested a review from our UI guru @tacigar as i'm not into the details at all on anything typescript related.

Original comment this PR is based on: #3162 (comment)

@jcchavezs
Copy link
Contributor

I am curious about what happens when we have a local root that does not show the aimed span. Maybe we change the local root? cc @adriancole

@tacigar
Copy link
Member

tacigar commented Aug 28, 2020

Thanks, about for scrolling, an external library such as react-scroll may be helpful...
https://github.com/fisshy/react-scroll

@tacigar
Copy link
Member

tacigar commented Aug 30, 2020

Clicking a span twice will redisplay that span as the root.

reroot

Perhaps it would be easier to display this rerooted diagram than scrolling to the span...

@codefromthecrypt
Copy link
Member

very much agree that moving back from re-rooting should use the new anchor. I think we discussed moving back to an anchor in the first development of re-root, just we didn't have the fully formed idea for anchoring yet

@tacigar tacigar added the ui Zipkin UI label Sep 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui Zipkin UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants