-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] Replace react-inspector with custom TreeView implementation #17662
Conversation
No bundle size changes comparing 4bfc2e4...eb375bb |
499e58d
to
438da84
Compare
Was trying to look into it but after building locally. I get a random error saying next-server can't find trim-right. It doesn't even look like the file reported uses trim-right either. |
We had some weird issues with netlify reporting the same error (we don't use trim-right anywhere anymore). Try clearing your |
So this is due to a limitation with the TreeView. https://github.com/mui-org/material-ui/blob/master/packages/material-ui-lab/src/TreeView/TreeView.js#L45 It looks for nodeId on its immediate child. ObjectTreeItem has nodeId but passes a different nodeId to TreeItem which breaks things. |
d4309dd
to
7874c2e
Compare
I completely missed this response, thanks. I'll finish this PR over the week. Current version should have working keyboard navigation. |
7874c2e
to
62f4e5d
Compare
62f4e5d
to
3566b22
Compare
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.
It's so cool to see our tree view used here 😍. I have noticed the following:
- I think that we have an indent issue. Compare these two cases:
What do you think of adding an indent of the same size as the expand icon, for the leaf case?
- We don't have enough contrast on the number type to reach the AA level. However; given that it seems to use the same style as the code formatting, I suspect this change https://github.com/mui-org/material-ui/pull/18283/files#r344488449 will fix the problem.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f97ef62
to
60e408e
Compare
ObjectTreeItem -> ObjectEntry TreeLabel -> ObjectEntryLabel
Does not affect contrast-ratio
566eb39
to
dcf43d8
Compare
Use our own
TreeView
instead ofreact-inspector
for https://material-ui.com/customization/default-theme/ (i.e. dogfeeding).Current bugs:
keyboard navigation between items doesn't work (collapsing works but not up/down arrow). Any idea why @joshwooding ?TODO:
dark/light mode themesuse syntax highlighting theme[ ] property preview for root object(feature cut, not that useful)Feedback regarding
TreeView
API:defaultCollapseIcon
might be confusing: Does this mean the icon that is displayed when we can collapse or is this the icon which is displayed when the item is collapse i.e. when we can expand (the latter one is the case). I would suggest swapping the names so that you don't have to writedefaultCollapseIcon={<ExpandIcon />}
which adds some cognitive overload ("I have to use the icon indicating expand forcollapseIcon
")end icon
is using terminology unknown to me. Whileleaf icon
is just as "jargony" it's at least Math/CS jargon.