-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 multi-view troubleshooting guide for sentry dart #11308
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Bundle ReportChanges will increase total bundle size by 14.23MB (100.74%) ⬆️
ℹ️ *Bundle size includes cached data from a previous commit |
@martinhaintz pls see my latest comment and finish this pr |
@martinhaintz you need to merge in the latest changes from master, there has been a fix regarding the failing CI |
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.
left 2 comments but looks good overall
* master: (121 commits) Bump API schema to 1ce35106 (#11467) Bump API schema to fe37a3fa (#11465) Bump API schema to e9717315 (#11464) fix(remix): Add verify step to manual guide (#11463) Bump API schema to ff5880ca (#11457) docs(cron): Add note about luxon timezones (#11451) Bump API schema to f0e51113 (#11454) Add User Feedback alerts to main Alerts docs page (#11446) fix: Bun import (#11450) Bump API schema to d8309de0 (#11447) Update Intro to Sentry page (#11443) fix(releases): Document `release:latest` filter behaviour (#11423) Bump API schema to d6663c91 (#11442) feat(js): Document dataloaderIntegration released with 8.31.0 (#11416) docs(go): Add custom instrumentation page for tracing (#11429) ref(onboarding): Update word (#11440) feat(nuxt): Update server setup docs (#11415) Update docs on running getsentry locally (#11428) Callout to add .env to .gitignore #11430 fix(dart): beforeSend code snippet (#11197) ...
Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>
Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>
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.
Let's wait for one more reviewer to review this
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.
left some small tweaks and clarifying questions
- User interaction integration via the `SentryUserInteractionWidget` (which is part of the `SentryWidget`) | ||
- Window & Device events via the `WidgetsBindingIntegration` | ||
|
||
To prevent the `WidgetsBindingIntegration` from loading by default, you will need to remove the integration. |
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.
l
: is this necessary? the above states that WidgetsBindingIntegration
doesn't support multi-view, but it's unclear whether any of the above features need/should be deactivated
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.
I suppose we need to be more specific, Screenshot and UserInteraction integrations are not added by default so users should not add/use them when enabling multiview but WidgetsBindingIntegration is enabled by default in the SDK which then requires the user to manually disable it.
@martinhaintz do you know if it's possible from the SDK side to detect whether multiview is enabled or not? If yes it makes things easy and we can just auto-disable unsupported features
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.
Screenshot and UserInteraction integrations are not added by default
ah yes, that makes sense. yeah would be good to be a bit more explicit
Co-authored-by: Karl Heinz Struggl <kahest@users.noreply.github.com>
Co-authored-by: Karl Heinz Struggl <kahest@users.noreply.github.com>
DESCRIBE YOUR PR
Multi-view for web was introduced in Flutter 3.24. As the current sentry dart package is not 100% compatible with this feature I added a guide, how to use sentry with multi-view in Flutter web.
Flutter 3.24
Flutter Docs for multi-view
Help us prioritize incoming PRs by letting us know when the change needs to go live.
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: