-
Notifications
You must be signed in to change notification settings - Fork 48.9k
feat: static Components panel layout #33696
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
feat: static Components panel layout #33696
Conversation
43dc232
to
4ed6eb5
Compare
return this.isDescendantOf(parentId, descendant.parentID); | ||
} | ||
|
||
getIndexOfLowestDescendantElement(element: Element): number | null { |
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.
maybe clarify somehow this is not the "lowest" as in "deepest" but the bottom-most branch (if I understood it correctly)
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.
Yeah, lowest in a sense of the bottom-most "branch", if you look at it from UI perspective. I do agree that "deepest" is probably not the right word here.
4ed6eb5
to
1d82cf4
Compare
Changes from 6.1.3: * feat: static Components panel layout ([hoxyq](https://github.com/hoxyq) in [#33696](#33696)) * fix: support optionality of structured stack trace function name ([hoxyq](https://github.com/hoxyq) in [#33697](#33697)) * fix: rename bottom stack frame ([hoxyq](https://github.com/hoxyq) in [#33680](#33680))
FWIW I think a significant motivation of the previous approach was to be able to scroll the tree and have idea of where you're going. In either direction. I don't think it's obvious to the user that moving the selection alone would do that. Not saying I strictly oppose it but I wonder if there's a significant usability loss for deep trees like on Comet. Especially in the everything-is-expanded default mode. |
Previous approach doesn't scale well exactly for the apps like the ones with Comet. When I was thinking about it, I always end up with 2 choices:
If we do 1, how do we determine the offset of the node?
For large trees, yeah, I do agree with you, but I think we traded this "see while scrolling" benefit for persistent offset and simpler UI navigation. I don't think there is a significant usability loss for wide / deep trees with Comet or react-strict-dom. The tree representation is so huge that looking at the full representation rarely would generate any valuable insights. What users usually do is select an element on the screen and then work with a subtree. |
Summary
Follow-up to #33517.
With #33517, we now preserve at least some minimal indent. This actually doesn't work with the current setup, because we don't allow the container to overflow, so basically deeply nested elements will go off the screen.
With these changes, we completely change the approach:
Demo
Current public release
Screen.Recording.2025-07-04.at.09.23.24.mov
With #33517
Screen.Recording.2025-07-04.at.09.22.12.mov
This PR
Screen.Recording.2025-07-04.at.09.19.47.mov