Skip to content

Commit 86bda66

Browse files
authored
Inspector v2: Factor react components out of service definitions (#16754)
Previously there were still some react components that were defined "inline" in the service definitions. I'm factoring the bulk of these out in this PR for a few reasons: 1. Had feedback from @georginahalpern and @sebavan that it would be easier to understand/read/maintain if they were separated. 2. React fast refresh **only** works for components that are directly exported from a module. If they are defined as part of an object, or returned from a function, or anything other than a straight component export, fast refresh does not work. So making this the standard pattern will significantly improve the dev workflow. 3. It makes it much easier to re-use the bulk of the react components part of things. For example, if we have a second hierarchy like view for audio (separate from scene explorer, as @docEdub prototyped), or if other tabs (like status and debug) have a similar UX to properties with an accordion view, sections, etc. As part of this, the raw react components got moved to the components folder, and now what is in the services folder is the service definitions that mostly are just managing the service registration and service interactions.
1 parent 0211cce commit 86bda66

File tree

11 files changed

+482
-445
lines changed

11 files changed

+482
-445
lines changed

packages/dev/inspector-v2/src/services/panes/properties/mesh/meshAdvancedProperties.tsx renamed to packages/dev/inspector-v2/src/components/properties/meshAdvancedProperties.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import type { AbstractMesh } from "core/index";
33

44
import type { FunctionComponent } from "react";
55

6-
import { useInterceptObservable } from "../../../../hooks/instrumentationHooks";
7-
import { useObservableState } from "../../../../hooks/observableHooks";
6+
import { useInterceptObservable } from "../../hooks/instrumentationHooks";
7+
import { useObservableState } from "../../hooks/observableHooks";
88

99
export const MeshAdvancedProperties: FunctionComponent<{ entity: AbstractMesh }> = ({ entity: mesh }) => {
1010
// There is no observable for computeBonesUsingShaders, so we use an interceptor to listen for changes.

packages/dev/inspector-v2/src/services/panes/properties/mesh/meshGeneralProperties.tsx renamed to packages/dev/inspector-v2/src/components/properties/meshGeneralProperties.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { AbstractMesh } from "core/index";
33

44
import type { FunctionComponent } from "react";
55

6-
import { useObservableState } from "../../../../hooks/observableHooks";
6+
import { useObservableState } from "../../hooks/observableHooks";
77

88
export const MeshGeneralProperties: FunctionComponent<{ entity: AbstractMesh }> = ({ entity: mesh }) => {
99
// Use the observable to keep keep state up-to-date and re-render the component when it changes.
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
// eslint-disable-next-line import/no-internal-modules
2+
3+
import type { FunctionComponent } from "react";
4+
5+
import { Accordion, AccordionHeader, AccordionItem, AccordionPanel, Body1Strong, makeStyles, Subtitle1, tokens } from "@fluentui/react-components";
6+
import { useMemo, useState, type ComponentType } from "react";
7+
8+
export type PropertiesServiceSection = Readonly<{
9+
/**
10+
* A unique identity for the section, which can be referenced by section content.
11+
*/
12+
identity: symbol;
13+
14+
/**
15+
* An optional order for the section, relative to other sections.
16+
* Defaults to 0.
17+
*/
18+
order?: number;
19+
20+
/**
21+
* An optional flag indicating whether the section should be collapsed by default.
22+
* Defaults to false.
23+
*/
24+
collapseByDefault?: boolean;
25+
}>;
26+
27+
export type PropertiesServiceSectionContent<EntityT> = Readonly<{
28+
/**
29+
* A unique key for the the content.
30+
*/
31+
key: string;
32+
33+
/**
34+
* A predicate function that determines if the content is applicable to the given entity.
35+
*/
36+
predicate: (entity: unknown) => entity is EntityT;
37+
38+
/**
39+
* The content that is added to individual sections.
40+
*/
41+
content: readonly Readonly<{
42+
/**
43+
* The section this content belongs to.
44+
*/
45+
section: symbol;
46+
47+
/**
48+
* An optional order for the content within the section.
49+
* Defaults to 0.
50+
*/
51+
order?: number;
52+
53+
/**
54+
* The React component that will be rendered for this content.
55+
*/
56+
component: ComponentType<{ entity: EntityT }>;
57+
}>[];
58+
}>;
59+
60+
// eslint-disable-next-line @typescript-eslint/naming-convention
61+
const useStyles = makeStyles({
62+
rootDiv: {
63+
flex: 1,
64+
overflow: "hidden",
65+
display: "flex",
66+
flexDirection: "column",
67+
},
68+
placeholderDiv: {
69+
padding: `${tokens.spacingVerticalM} ${tokens.spacingHorizontalM}`,
70+
},
71+
accordion: {
72+
overflowY: "auto",
73+
paddingBottom: tokens.spacingVerticalM,
74+
},
75+
panelDiv: {
76+
display: "flex",
77+
flexDirection: "column",
78+
overflow: "hidden",
79+
},
80+
});
81+
82+
export const PropertiesPane: FunctionComponent<{
83+
sections: readonly PropertiesServiceSection[];
84+
sectionContent: readonly PropertiesServiceSectionContent<unknown>[];
85+
entity: unknown;
86+
}> = (props) => {
87+
const classes = useStyles();
88+
89+
const { sections, sectionContent, entity } = props;
90+
91+
const [version, setVersion] = useState(0);
92+
93+
const visibleSections = useMemo(() => {
94+
// When any of this state changes, we should re-render the Accordion so the defaultOpenItems are re-evaluated.
95+
setVersion((prev) => prev + 1);
96+
97+
if (!entity) {
98+
return [];
99+
}
100+
101+
const applicableContent = sectionContent.filter((content) => content.predicate(entity));
102+
return sections
103+
.map((section) => {
104+
// Get a flat list of the section content, preserving the key so it can be used when each component for each section is rendered.
105+
const contentForSection = applicableContent
106+
.flatMap((entry) => entry.content.map((content) => ({ key: entry.key, ...content })))
107+
.filter((content) => content.section === section.identity);
108+
109+
// If there is no content for this section, we skip it.
110+
if (contentForSection.length === 0) {
111+
return null; // No content for this section
112+
}
113+
114+
// Sort the content for this section by order, defaulting to 0 if not specified.
115+
contentForSection.sort((a, b) => (a.order ?? 0) - (b.order ?? 0));
116+
117+
// Return the section with its identity, collapseByDefault flag, and the content components to render.
118+
return {
119+
identity: section.identity,
120+
collapseByDefault: section.collapseByDefault ?? false,
121+
components: contentForSection.map((content) => ({ key: content.key, component: content.component })),
122+
};
123+
})
124+
.filter((section) => section !== null);
125+
}, [sections, sectionContent, entity]);
126+
127+
return (
128+
<div className={classes.rootDiv}>
129+
{visibleSections.length > 0 ? (
130+
<Accordion
131+
key={version}
132+
className={classes.accordion}
133+
collapsible
134+
multiple
135+
defaultOpenItems={visibleSections.filter((section) => !section.collapseByDefault).map((section) => section.identity.description)}
136+
>
137+
{visibleSections.map((section) => {
138+
return (
139+
<AccordionItem key={section.identity.description} value={section.identity.description}>
140+
<AccordionHeader expandIconPosition="end">
141+
<Subtitle1>{section.identity.description}</Subtitle1>
142+
</AccordionHeader>
143+
<AccordionPanel>
144+
<div className={classes.panelDiv}>
145+
{section.components.map((component) => {
146+
return <component.component key={component.key} entity={entity} />;
147+
})}
148+
</div>
149+
</AccordionPanel>
150+
</AccordionItem>
151+
);
152+
})}
153+
</Accordion>
154+
) : (
155+
<div className={classes.placeholderDiv}>
156+
<Body1Strong italic>{entity ? `Can't show properties for the selected entity type (${entity.toString()})` : "No entity selected."}</Body1Strong>
157+
</div>
158+
)}
159+
</div>
160+
);
161+
};

0 commit comments

Comments
 (0)