-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add select-all controls in property panel #176
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
base: master
Are you sure you want to change the base?
feat: add select-all controls in property panel #176
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughImplements a path-based “Select All” action in ModelInfo. Adds selectElementsByProperty(path, value) to IFC context and wires it to UI controls beside property values (attributes, property sets, type sets, IFC Class). Introduces i18n key selectAll across locales. Minor layout tweaks to accommodate the new control. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant MI as ModelInfo UI
participant PR as PropertyRow
participant IFCCtx as IFC Context
participant Cache as PropertyCache
participant Sel as Selection Helper
U->>MI: Click "Select all" icon
MI->>PR: handleSelect(propertyPath, value)
PR->>IFCCtx: selectElementsByProperty(path, value)
alt path == ["ifcType"]
IFCCtx->>IFCCtx: Compare element.type to value (case-insensitive)
else Other property path
IFCCtx->>Cache: Get element properties
IFCCtx->>IFCCtx: Traverse path to extract value
IFCCtx->>IFCCtx: Deep-equal compare to target
end
IFCCtx->>Sel: selectElements(matched [{modelID, expressID}])
Sel-->>U: Elements highlighted/selected
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/model-info.tsx (1)
86-97: Fix array rendering: joining ReactNodes produces “[object Object]”Both unit-wrapped arrays and simple arrays call
.join(", ")on ReactNodes. Render with Fragments instead.- if (Array.isArray(value.values) && value.unit !== undefined) { - // Handles { values: [...], unit: Y } - const displayValues = value.values - .map((v: any) => renderPropertyValue(v, keyHint, ifcApi, t)) // Recursively render each value in the array - .join(", "); - return ( - <> - {displayValues}{" "} - <span className="text-muted-foreground/80">({value.unit})</span> - </> - ); - } + if (Array.isArray(value.values) && value.unit !== undefined) { + // Handles { values: [...], unit: Y } + return ( + <> + {value.values.map((v: any, i: number) => ( + <React.Fragment key={i}> + {i > 0 && ", "} + {renderPropertyValue(v, keyHint, ifcApi, t)} + </React.Fragment> + ))}{" "} + <span className="text-muted-foreground/80">({value.unit})</span> + </> + ); + } @@ - if ( + if ( value.length <= 5 && // Allow slightly longer lists if they are simple value.every( (v) => typeof v === "string" || typeof v === "number" || typeof v === "boolean", ) ) { - return value.map((v) => renderPropertyValue(v, keyHint, ifcApi, t)).join(", "); // Render each item + return ( + <> + {value.map((v, i) => ( + <React.Fragment key={i}> + {i > 0 && ", "} + {renderPropertyValue(v, keyHint, ifcApi, t)} + </React.Fragment> + ))} + </> + ); }Also applies to: 171-190
🧹 Nitpick comments (6)
public/locales/en/common.json (1)
28-28: Grammar nit: add “the”.
Prefer “Select all with the same value”.Apply this diff:
- "selectAll": "Select all with same value", + "selectAll": "Select all with the same value",context/ifc-context.tsx (3)
147-154: Clarify path/value contract in the JSDoc.
Mention that leaf values may be wrapped as { value } and whether string comparisons are case-insensitive. This prevents mismatches across locales and sources.
1574-1574: Guard against nullish intermediates in path traversal.
Use explicit null/undefined checks instead of truthiness to avoid skipping valid falsy values.Apply this diff:
- const getValue = (obj: any, p: string[]) => p.reduce((acc, key) => (acc ? acc[key] : undefined), obj); + const getValue = (obj: any, p: string[]) => + p.reduce((acc, key) => (acc !== undefined && acc !== null ? (acc as any)[key] : undefined), obj);
1569-1573: Initialize api.properties for non-IfcType paths and skip closed models
InselectElementsByProperty, before usingifcApiInternal.properties, initialize it when the path isn’t["ifcType"], and after themodel.modelID == nullcheck, skip models whereIsModelOpenreturns false:async (path: string[], value: any) => { if (!ifcApiInternal) return; + if ((path.length !== 1 || path[0] !== "ifcType") && !ifcApiInternal.properties) { + try { ifcApiInternal.properties = new Properties(ifcApiInternal); } catch {} + } const matches: SelectedElementInfo[] = []; for (const model of models) { - if (model.modelID == null) continue; + if (model.modelID == null) continue; + if (typeof ifcApiInternal.IsModelOpen === "function" && + !ifcApiInternal.IsModelOpen(model.modelID)) continue; // … } }components/model-info.tsx (2)
206-207: propertyPath prop introduction — good; consider readonly typingChange to readonly for safer intent and better TS inference.
-interface PropertyRowProps { +interface PropertyRowProps { propKey: string; propValue: any; icon?: React.ReactNode; copyValue?: string; t?: (key: string, options?: any) => string; - propertyPath?: string[]; + propertyPath?: readonly string[]; } const PropertyRow: React.FC<PropertyRowProps> = ({ propKey, propValue, icon, copyValue, t, - propertyPath, + propertyPath, }) => { - const { ifcApi, selectElementsByProperty } = useIFCContext(); + const { ifcApi, selectElementsByProperty } = useIFCContext();Also applies to: 215-216, 217-217
221-225: Normalize values before selection to avoid non-primitive comparisonsSelection likely expects primitives; unit-wrapped or typed objects may fail equality.
- const handleSelect = async () => { - if (propertyPath) { - await selectElementsByProperty(propertyPath, propValue); - } - }; + const handleSelect = async () => { + if (!propertyPath) return; + // unwrap common shapes: { value, type } or { value, unit } + const v = + propValue && typeof propValue === "object" && "value" in propValue + ? (propValue as any).value + : propValue; + await selectElementsByProperty(propertyPath, v); + };Please confirm selectElementsByProperty supports non-primitive values; if needed we can add a shared normalizer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/model-info.tsx(11 hunks)context/ifc-context.tsx(3 hunks)public/locales/de/common.json(1 hunks)public/locales/en/common.json(1 hunks)public/locales/fr/common.json(1 hunks)public/locales/it/common.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
**/*.{ts,tsx,js,jsx}: If using TypeScript, use an enum to store flag names. If using JavaScript, store flag names as strings to an object declared as a constant, to simulate an enum. Use a consistent naming convention for this storage. enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
Gate flag-dependent code on a check that verifies the flag's values are valid and expected.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
components/model-info.tsxcontext/ifc-context.tsx
🧬 Code graph analysis (2)
components/model-info.tsx (1)
context/ifc-context.tsx (1)
useIFCContext(2401-2407)
context/ifc-context.tsx (2)
services/ifc-element-extractor.ts (1)
IFCElementExtractor(12-417)services/property-cache.ts (1)
PropertyCache(7-217)
🔇 Additional comments (5)
public/locales/de/common.json (1)
28-28: LGTM — translation reads naturally.
"Alle mit gleichem Wert auswählen" is clear and idiomatic.public/locales/it/common.json (1)
28-28: LGTM — Italian string looks good.
"Seleziona tutti con lo stesso valore" is natural and consistent with the UI.context/ifc-context.tsx (1)
2349-2349: Context API exposure looks correct.
Method is added to the provider value and matches the interface.components/model-info.tsx (2)
388-389: Context wiring for selectElementsByProperty looks correctNo issues with dependency usage.
701-702: Verify path contract and special characters in keysConfirm the selector expects these prefixes and raw keys:
- ["attributes", ...]
- ["propertySets", psetName, propName]
- ["typeSets", psetName, propName]
If psetName/propName can contain spaces, colons (“Type Attributes: …”), or parentheses, ensure the backend matcher treats path segments as opaque strings (no splitting).
Also applies to: 712-713, 723-724, 754-755, 837-838, 873-874
| {propertyPath && ( | ||
| <TooltipProvider delayDuration={100}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={handleSelect} | ||
| className="opacity-60 hover:opacity-100" | ||
| > | ||
| <MousePointer2 className="w-3 h-3" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| {t ? t("selectAll") : "Select all"} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} |
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.
🛠️ Refactor suggestion
Icon button needs a11y: add type, aria-label, and focus styles
Improve accessibility and prevent accidental form submission.
- {propertyPath && (
+ {propertyPath && (
<TooltipProvider delayDuration={100}>
<Tooltip>
<TooltipTrigger asChild>
- <button
- onClick={handleSelect}
- className="opacity-60 hover:opacity-100"
- >
- <MousePointer2 className="w-3 h-3" />
- </button>
+ <button
+ type="button"
+ onClick={handleSelect}
+ aria-label={t ? t("selectAll") : "Select all"}
+ title={t ? t("selectAll") : "Select all"}
+ className="opacity-60 hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded"
+ >
+ <MousePointer2 className="w-3 h-3" aria-hidden="true" />
+ </button>
</TooltipTrigger>
<TooltipContent>
{t ? t("selectAll") : "Select all"}
</TooltipContent>
</Tooltip>
</TooltipProvider>
- )}
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {propertyPath && ( | |
| <TooltipProvider delayDuration={100}> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <button | |
| onClick={handleSelect} | |
| className="opacity-60 hover:opacity-100" | |
| > | |
| <MousePointer2 className="w-3 h-3" /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| {t ? t("selectAll") : "Select all"} | |
| </TooltipContent> | |
| </Tooltip> | |
| </TooltipProvider> | |
| )} | |
| {propertyPath && ( | |
| <TooltipProvider delayDuration={100}> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <button | |
| type="button" | |
| onClick={handleSelect} | |
| aria-label={t ? t("selectAll") : "Select all"} | |
| title={t ? t("selectAll") : "Select all"} | |
| className="opacity-60 hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded" | |
| > | |
| <MousePointer2 className="w-3 h-3" aria-hidden="true" /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| {t ? t("selectAll") : "Select all"} | |
| </TooltipContent> | |
| </Tooltip> | |
| </TooltipProvider> | |
| )} |
🤖 Prompt for AI Agents
components/model-info.tsx around lines 243 to 259: the icon button lacks an
explicit type, aria-label, and visible focus styles which can cause accidental
form submission and poor keyboard accessibility; set the button's type="button",
add an aria-label that uses the translated label (e.g., aria-label={t ?
t("selectAll") : "Select all"}) to provide an accessible name, and add
focus/focus-visible utility classes (e.g., focus:outline-none
focus-visible:ring-2 focus-visible:ring-offset-1 or your design system's focus
classes) so keyboard focus is clearly visible.
| {ifcType && ( | ||
| <TooltipProvider delayDuration={100}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={() => | ||
| selectElementsByProperty(['ifcType'], ifcType) | ||
| } | ||
| className="opacity-60 hover:opacity-100" | ||
| > | ||
| <MousePointer2 className="w-3 h-3" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| {t('selectAll')} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} |
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.
🛠️ Refactor suggestion
Add a11y to IFC Class select-all control
Mirror the PropertyRow fix: type, aria-label, focus ring.
- <button
- onClick={() =>
- selectElementsByProperty(['ifcType'], ifcType)
- }
- className="opacity-60 hover:opacity-100"
- >
- <MousePointer2 className="w-3 h-3" />
- </button>
+ <button
+ type="button"
+ onClick={() => selectElementsByProperty(['ifcType'], ifcType)}
+ aria-label={t('selectAll')}
+ title={t('selectAll')}
+ className="opacity-60 hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded"
+ >
+ <MousePointer2 className="w-3 h-3" aria-hidden="true" />
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {ifcType && ( | |
| <TooltipProvider delayDuration={100}> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <button | |
| onClick={() => | |
| selectElementsByProperty(['ifcType'], ifcType) | |
| } | |
| className="opacity-60 hover:opacity-100" | |
| > | |
| <MousePointer2 className="w-3 h-3" /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| {t('selectAll')} | |
| </TooltipContent> | |
| </Tooltip> | |
| </TooltipProvider> | |
| )} | |
| {ifcType && ( | |
| <TooltipProvider delayDuration={100}> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| - <button | |
| - onClick={() => | |
| - selectElementsByProperty(['ifcType'], ifcType) | |
| - } | |
| - className="opacity-60 hover:opacity-100" | |
| - > | |
| - <MousePointer2 className="w-3 h-3" /> | |
| <button | |
| type="button" | |
| onClick={() => selectElementsByProperty(['ifcType'], ifcType)} | |
| aria-label={t('selectAll')} | |
| title={t('selectAll')} | |
| className="opacity-60 hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded" | |
| > | |
| <MousePointer2 className="w-3 h-3" aria-hidden="true" /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| {t('selectAll')} | |
| </TooltipContent> | |
| </Tooltip> | |
| </TooltipProvider> | |
| )} |
🤖 Prompt for AI Agents
In components/model-info.tsx around lines 624–642, the IFC Class select-all
button is missing accessibility attributes and focus styling; update the button
to include type="button", an appropriate aria-label (e.g.
aria-label={t('selectAll')} or a more specific translated label for IFC Class),
and add the same focus ring classes used in PropertyRow (for example
focus:ring-2 focus:ring-offset-2 focus:ring-primary or your project's
equivalent) so it is keyboard-focusable and announced correctly by screen
readers.
| for (const el of elements) { | ||
| try { | ||
| const props = await PropertyCache.getProperties( | ||
| ifcApiInternal, | ||
| model.modelID, | ||
| el.expressID, | ||
| ); | ||
| const v = getValue(props, path); | ||
| if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) { | ||
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | ||
| } | ||
| } catch (e) { | ||
| // ignore errors for individual elements | ||
| } | ||
| } |
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.
🛠️ Refactor suggestion
O(N) sequential awaits will freeze the UI on large models. Batch property fetches and yield to the main thread.
You’re awaiting PropertyCache.getProperties inside a tight loop; with tens of thousands of elements this can take seconds and block interactivity.
Apply this refactor of the whole function to:
- use getBatchProperties in chunks,
- normalize/unwrap values,
- yield between chunks,
- check IsModelOpen,
- init api.properties when needed.
- const selectElementsByProperty = useCallback(
- async (path: string[], value: any) => {
- if (!ifcApiInternal) return;
- const matches: SelectedElementInfo[] = [];
-
- const getValue = (obj: any, p: string[]) => p.reduce((acc, key) => (acc ? acc[key] : undefined), obj);
-
- for (const model of loadedModels) {
- if (model.modelID == null) continue;
- const elements = IFCElementExtractor.getAllElements(ifcApiInternal, model.modelID);
-
- if (path.length === 1 && path[0] === "ifcType") {
- for (const el of elements) {
- if (String(el.type).toLowerCase() === String(value).toLowerCase()) {
- matches.push({ modelID: model.modelID, expressID: el.expressID });
- }
- }
- } else {
- for (const el of elements) {
- try {
- const props = await PropertyCache.getProperties(
- ifcApiInternal,
- model.modelID,
- el.expressID,
- );
- const v = getValue(props, path);
- if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) {
- matches.push({ modelID: model.modelID, expressID: el.expressID });
- }
- } catch (e) {
- // ignore errors for individual elements
- }
- }
- }
- }
-
- if (matches.length) {
- selectElements(matches);
- }
- },
- [ifcApiInternal, loadedModels, selectElements],
- );
+ const selectElementsByProperty = useCallback(
+ async (path: string[], value: any) => {
+ if (!ifcApiInternal) return;
+
+ // Ensure properties helper is present when traversing props
+ if ((path.length !== 1 || path[0] !== "ifcType") && !ifcApiInternal.properties) {
+ try { ifcApiInternal.properties = new Properties(ifcApiInternal); } catch {}
+ }
+
+ const matches: SelectedElementInfo[] = [];
+ const getValue = (obj: any, p: string[]) =>
+ p.reduce((acc, key) => (acc !== undefined && acc !== null ? (acc as any)[key] : undefined), obj);
+ const unwrap = (v: any) => (v && typeof v === "object" && "value" in v ? (v as any).value : v);
+ const normalize = (v: any) => {
+ const un = unwrap(v);
+ return typeof un === "string" ? un.trim().toLowerCase() : un;
+ };
+ const deepEqNormalized = (a: any, b: any) => {
+ const na = normalize(a);
+ const nb = normalize(b);
+ if (typeof na === "string" && typeof nb === "string") return na === nb;
+ try { return JSON.stringify(na) === JSON.stringify(nb); } catch { return na === nb; }
+ };
+ const normTarget = normalize(value);
+
+ for (const model of loadedModels) {
+ if (model.modelID == null) continue;
+ if (typeof (ifcApiInternal as any).IsModelOpen === "function" &&
+ !(ifcApiInternal as any).IsModelOpen(model.modelID)) continue;
+
+ const elements = IFCElementExtractor.getAllElements(ifcApiInternal, model.modelID);
+
+ // Fast path for IFC type
+ if (path.length === 1 && path[0] === "ifcType") {
+ for (const el of elements) {
+ if (normalize(el.type) === normTarget) {
+ matches.push({ modelID: model.modelID, expressID: el.expressID });
+ }
+ }
+ continue;
+ }
+
+ // Batch-fetch properties
+ const CHUNK = 100;
+ for (let i = 0; i < elements.length; i += CHUNK) {
+ const chunk = elements.slice(i, i + CHUNK);
+ const ids = chunk.map(e => e.expressID);
+ const propsMap = await PropertyCache.getBatchProperties(ifcApiInternal, model.modelID, ids);
+
+ for (const id of ids) {
+ const props = propsMap.get(id);
+ if (!props) continue;
+ const v = getValue(props, path);
+ if (v !== undefined && deepEqNormalized(v, value)) {
+ matches.push({ modelID: model.modelID, expressID: id });
+ }
+ }
+
+ // Yield to keep UI responsive on large selections
+ // eslint-disable-next-line no-await-in-loop
+ await new Promise((r) => setTimeout(r, 0));
+ }
+ }
+
+ if (matches.length) {
+ selectElements(matches);
+ }
+ },
+ [ifcApiInternal, loadedModels, selectElements],
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const el of elements) { | |
| try { | |
| const props = await PropertyCache.getProperties( | |
| ifcApiInternal, | |
| model.modelID, | |
| el.expressID, | |
| ); | |
| const v = getValue(props, path); | |
| if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) { | |
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | |
| } | |
| } catch (e) { | |
| // ignore errors for individual elements | |
| } | |
| } | |
| const selectElementsByProperty = useCallback( | |
| async (path: string[], value: any) => { | |
| if (!ifcApiInternal) return; | |
| // Ensure properties helper is present when traversing props | |
| if ((path.length !== 1 || path[0] !== "ifcType") && !ifcApiInternal.properties) { | |
| try { ifcApiInternal.properties = new Properties(ifcApiInternal); } catch {} | |
| } | |
| const matches: SelectedElementInfo[] = []; | |
| const getValue = (obj: any, p: string[]) => | |
| p.reduce((acc, key) => (acc !== undefined && acc !== null ? (acc as any)[key] : undefined), obj); | |
| const unwrap = (v: any) => (v && typeof v === "object" && "value" in v ? (v as any).value : v); | |
| const normalize = (v: any) => { | |
| const un = unwrap(v); | |
| return typeof un === "string" ? un.trim().toLowerCase() : un; | |
| }; | |
| const deepEqNormalized = (a: any, b: any) => { | |
| const na = normalize(a); | |
| const nb = normalize(b); | |
| if (typeof na === "string" && typeof nb === "string") return na === nb; | |
| try { return JSON.stringify(na) === JSON.stringify(nb); } catch { return na === nb; } | |
| }; | |
| const normTarget = normalize(value); | |
| for (const model of loadedModels) { | |
| if (model.modelID == null) continue; | |
| // Skip models that have been closed | |
| if (typeof (ifcApiInternal as any).IsModelOpen === "function" && | |
| !(ifcApiInternal as any).IsModelOpen(model.modelID)) continue; | |
| const elements = IFCElementExtractor.getAllElements(ifcApiInternal, model.modelID); | |
| // Fast path for IFC type | |
| if (path.length === 1 && path[0] === "ifcType") { | |
| for (const el of elements) { | |
| if (normalize(el.type) === normTarget) { | |
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | |
| } | |
| } | |
| continue; | |
| } | |
| // Batch-fetch properties in chunks and yield between them | |
| const CHUNK = 100; | |
| for (let i = 0; i < elements.length; i += CHUNK) { | |
| const chunk = elements.slice(i, i + CHUNK); | |
| const ids = chunk.map(e => e.expressID); | |
| const propsMap = await PropertyCache.getBatchProperties(ifcApiInternal, model.modelID, ids); | |
| for (const id of ids) { | |
| const props = propsMap.get(id); | |
| if (!props) continue; | |
| const v = getValue(props, path); | |
| if (v !== undefined && deepEqNormalized(v, value)) { | |
| matches.push({ modelID: model.modelID, expressID: id }); | |
| } | |
| } | |
| // Yield to keep UI responsive on large selections | |
| // eslint-disable-next-line no-await-in-loop | |
| await new Promise((r) => setTimeout(r, 0)); | |
| } | |
| } | |
| if (matches.length) { | |
| selectElements(matches); | |
| } | |
| }, | |
| [ifcApiInternal, loadedModels, selectElements], | |
| ); |
🤖 Prompt for AI Agents
In context/ifc-context.tsx around lines 1587 to 1601, the current code awaits
PropertyCache.getProperties sequentially inside a loop which blocks the UI on
large models; replace the per-element sequential awaits with chunked calls to
PropertyCache.getBatchProperties (fetch properties for many expressIDs at once),
ensure api.properties is initialized before use, normalize/unwrap property
values before comparison (so JSON-equal checks are reliable), yield to the main
thread between chunks (e.g., await a microtask/frame) and check IsModelOpen
after each yield to abort if the model closed, and finally collect matches by
comparing normalized values and pushing {modelID, expressID} for hits instead of
awaiting per-element.
| const v = getValue(props, path); | ||
| if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) { | ||
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | ||
| } |
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.
Functional mismatch: comparing wrapped values via JSON may miss { value } leaves.
IFC props often come as objects like { value: "X" }. Comparing JSON directly to a primitive will fail.
The refactor above unwraps and normalizes leaf values before comparison. If you keep the current approach, at minimum unwrap:
- const v = getValue(props, path);
- if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) {
+ let v = getValue(props, path);
+ if (v && typeof v === "object" && "value" in v) v = (v as any).value;
+ if (v !== undefined && String(v).trim().toLowerCase() === String(value).trim().toLowerCase()) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const v = getValue(props, path); | |
| if (v !== undefined && JSON.stringify(v) === JSON.stringify(value)) { | |
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | |
| } | |
| let v = getValue(props, path); | |
| if (v && typeof v === "object" && "value" in v) v = (v as any).value; | |
| if (v !== undefined && String(v).trim().toLowerCase() === String(value).trim().toLowerCase()) { | |
| matches.push({ modelID: model.modelID, expressID: el.expressID }); | |
| } |
🤖 Prompt for AI Agents
In context/ifc-context.tsx around lines 1594-1597, the code compares
getValue(props, path) to value using JSON.stringify which fails when leaves are
wrapped as objects like { value: "X" }; normalize both sides by unwrapping such
wrapper objects (if the result is an object with a 'value' property, use that
inner value) and then compare the normalized values (for primitives use ===, for
non-primitives use a proper deep equality check or stable JSON serialization)
and push the match when they are equal.
| "complexData": "Données complexes", | ||
| "loadingSchemaPreview": "Chargement de l'aperçu du schéma...", | ||
| "clickToExploreFullDocumentation": "Cliquez pour explorer la documentation complète", | ||
| "selectAll": "Sélectionner tous avec la même valeur", |
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.
Fix French grammar for tooltip
Use “tous les éléments” instead of “tous”.
Apply this diff:
- "selectAll": "Sélectionner tous avec la même valeur",
+ "selectAll": "Sélectionner tous les éléments avec la même valeur",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "selectAll": "Sélectionner tous avec la même valeur", | |
| "selectAll": "Sélectionner tous les éléments avec la même valeur", |
🤖 Prompt for AI Agents
In public/locales/fr/common.json around line 28, the French tooltip text uses
"Sélectionner tous" which is grammatically incomplete; replace the value with
the corrected phrase "Sélectionner tous les éléments avec la même valeur" so the
tooltip reads fully and correctly.
Summary
selectElementsByPropertyto context for selecting elements sharing a property valueTesting
npm test(fails: Missing script: "test")npm run linthttps://chatgpt.com/codex/tasks/task_e_68bfcd38ae2c832083cb631a050cd0b7
Summary by CodeRabbit
New Features
UI
Localization