Use rendering_system to enable absolute positioning of view hierarchy wireframe
#83292
Conversation
github-actions
bot
added
the
Scope: Frontend
timfish
changed the title
rendering_system to enable absolute positioning of view hierarchy layoutrendering_system to enable absolute positioning of view hierarchy wireframe
| getHierarchyDimensions( | ||
| hierarchy, | ||
| ['flutter', 'dart-flutter'].includes(project?.platform ?? '') | ||
| system === 'flutter' || system.includes('absolute') |
There was a problem hiding this comment.
Just want to make sure that flutter and dart-flutter projects stay absolutely positioned with this change, do they both have system set to flutter? I agree with removing the project platform check though, good call!
There was a problem hiding this comment.
do they both have
systemset toflutter?
Yep, it looks like sentry-dart uses flutter for the rendering_system too.
https://github.com/getsentry/sentry-dart/blob/ec50b2159d85b69bbbb3619960b3f922ec5592f5/dart/test/protocol/sentry_view_hierarchy_test.dart#L25
who could we ping to triple check?
There was a problem hiding this comment.
Thanks for checking that out, I think the last step for this is to have in docs or mentioned somewhere that to use absolute rendering in the UI, the rendering_system must contain absolute, but obviously that is outside the scope of this PR. Approving!
There was a problem hiding this comment.
I will PR the original RFC because it doesn't look like this is documented elsewhere yet
There was a problem hiding this comment.
I'm working on Flutter, yes the rendering_system is always set to flutter: https://github.com/getsentry/sentry-dart/blob/ea6d86d569f24c0dd33f13fe51baa9b53c770d2a/flutter/lib/src/view_hierarchy/sentry_tree_walker.dart#L242
so looks good!
There was a problem hiding this comment.
I'll also update the rendering system to absolute on flutter
There was a problem hiding this comment.
So it would become something like flutter-absolute?
There was a problem hiding this comment.
I think we might need to be careful about changing existing rendering_system because the Sentry frontend uses for icons or something 🤔
There was a problem hiding this comment.
Oh, it looks like platform is used for the icon:
However, the system does end up in the tooltip and we might want to hide the absolute detail from users:
There was a problem hiding this comment.
yup, I'll double check there, it's rather low prio so I won't change it immediately, just created an issue for it in the dart repo
leeandher
added
the
Trigger: getsentry tests
|
Rather than adding |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
3 participants
rendering_systemrather thanproject.platform#83291Let me know if you want to retain the previous project check too...