-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
fix: Updated breadcrumb type for scene changes #2028
base: main
Are you sure you want to change the base?
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Updated breadcrumb type for scene changes ([#2028](https://github.com/getsentry/sentry-unity/pull/2028)) If none of the above apply, you can opt out of this check by adding |
@@ -29,7 +29,8 @@ void OnSceneManagerOnSceneLoaded(SceneAdapter scene, LoadSceneMode mode) | |||
|
|||
hub.AddBreadcrumb( | |||
message: $"Scene '{scene.Name}' was loaded", | |||
category: "scene.loaded"); | |||
category: "scene.loaded", | |||
type: "navigation"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigation breadcrumbs should also have data
. From the develop docs:
navigation
A navigation event can be a URL change in a web application, or a UI transition in a mobile or desktop application, etc.
Internally we display breadcrumbs of type default that contain category navigation as a breadcrumb of type navigation.
Its data property has the following sub-properties:
from (Required)
A string representing the original application state / location.
to (Required)
A string representing the new application state / location.
{
"type": "navigation",
"category": "navigation",
"timestamp": "2016-04-20T20:55:53.845Z",
"data": {
"from": "/login",
"to": "/dashboard"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking out loud here: You can load scenes additionally, how'd that work with the from
->to
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be current
vs previous
? it can be anything that makes sense here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then, I guess it's not a navigation event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd actually make sense. As in: Which scene cause which other scene to load. That'd be actually super useful in specifically that case. How else would you know where this scene is coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - doesn't work. The fromScene
in the ActiveSceneChanged
call is always null
. In all Unity versions. So all the change to type
really does is change the icon.
This helps (visually) to distinguish the breadcrumb's type for scene loading.
Before
After