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

Add text to elements in breadcrumbs ui.click #577

Closed
soroushhakami opened this issue May 12, 2016 · 18 comments
Closed

Add text to elements in breadcrumbs ui.click #577

soroushhakami opened this issue May 12, 2016 · 18 comments

Comments

@soroushhakami
Copy link

It'd make things easier especially for apps where the classnames don't reveal much.

Something along the lines of :

 div > button.btn.btn-lg.btn-primary "Purchase"

If the element contains alot of text, it could be truncated.

@benvinegar
Copy link
Contributor

So, this is something I'd like to do, but I'm concerned about bloating our DOM serializing function to the point that it begins to add noticeable overhead to click handlers.

But, I will explore.

@soroushhakami
Copy link
Author

soroushhakami commented May 17, 2016

One easy alternative could be to just do

target = elem.outerHTML;

that will output the element clicked exactly as is. You'll loose the tree structure - but in my personal experience that doesn't help as much as seeing the full, actual element that I'm used to seeing in my code all the time and therefore more quickly can recognize.

That would solve #576 as well

@benvinegar
Copy link
Contributor

@soroushhakami – we started with outerHTML first but deprecated it for this utility method.

The reason we didn't go with outerHTML (as @mitsuhiko commented on):

  • the CSS selector format can show more information with less characters
  • outerHTML can include noisy temporary data, e.g.
    • long data-reactid strings (in React 0.14 and earlier)
    • dynamically generated title attributes intended for tooltips
  • we would get a lot of empty elements, depending on how the addEventListener target is configured
  • we had buttons w/ the same class / content, that are only contextualized via ancestor information

Basically, outerHTML seems nice when you show a bunch of hypothetical examples where it looks nice, but our experience of running it live for a few weeks was that it was insufficient.

@soroushhakami
Copy link
Author

Ah I see, makes sense, thanks for the explanation! 👍

@nblasgen
Copy link

I've added ID tags to all my buttons. Surprised that they don't show up in the breadcrumbs. Even if it was a custom tag, I'd be happy with it. But ID tag seems to be ideal due to its uniqueness.

@benvinegar
Copy link
Contributor

@nblasgen – if you can provide a failing test case, I'd be glad to look at what's up.

But right now the code is supposed to capture IDs, and there is a test that verifies this.

@adamreisnz
Copy link

+1 for showing ID's, or a custom tag like data-sentry-id="Some text here"

@kamilogorek
Copy link
Contributor

I'll close this one for now, as we're aware of this missing piece and we'll be revisiting how this thing is working in the next major version.

Ref: #783
Ref: #576

@alexlouden
Copy link

Just wanted to add - I'm using react & styled-components so my class names are automatically generated, would be great to have some more context (without having to manually captureBreadcrumb):

screen shot 2018-07-02 at 4 36 14 pm

@tkharuk
Copy link

tkharuk commented Jul 20, 2018

@kamilogorek any updates on this?

Is there a possibility to fine-tune click "message"?

Maybe, more data can be passed into breadcrumbCallback for people to pick what they need without sending it to sentry by default? Maybe even a whole dom element?

@kamilogorek
Copy link
Contributor

@TuxujPes it'll be possible in the next major release, however not in this one, as it requires some public API changes that we can only introduce in major releases.

@tkharuk
Copy link

tkharuk commented Aug 2, 2018

@kamilogorek that sounds great. Any estimations on major release date?

@kamilogorek
Copy link
Contributor

Somewhere near the end of Q3 (excuse me such a long response time, was on vacations).

@hiagodotme
Copy link

hiagodotme commented Oct 3, 2018

+1 for showing ID's, or a custom tag like data-sentry-id="Some text here"

They could analyze if there is this information and take it along. If it exists, leave the selectors with a button for more information.

@kamilogorek
Copy link
Contributor

@hiagodotme @TuxujPes @adamreisnz this feature is already available in new SDK - https://docs.sentry.io/learn/filtering/?platform=javascript#before-breadcrumb

@1999
Copy link
Contributor

1999 commented Mar 14, 2019

FYI: just don't forget that using new SDK brings you +50Kb for the bundle size #1552

@kamilogorek
Copy link
Contributor

@1999 it add's 12.7kB when compared to raven-js, don't use non-gzipped sizes for comparison, as it's not relevant.
Also v5 (#1919) is going to be <15kB, so you won't have to worry about your payload increase.

@1999
Copy link
Contributor

1999 commented Mar 15, 2019

@kamilogorek I'm a bit afraid of PRs with more than 200 changes files, this seems more like a completely new SDK rather than a new version. Also, what's the ETA release date of it?

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

No branches or pull requests

9 participants