DOPE-60 | DOPE-61#285
Merged
Gustavohsdp merged 3 commits intoJul 22, 2025
Merged
Conversation
Contributor
|
""" WalkthroughThe changes refactor type selection UIs in various components by replacing flat dropdowns with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TypeDropdownSelector
participant ParentComponent
User->>TypeDropdownSelector: Clicks dropdown trigger
TypeDropdownSelector->>User: Shows categorized type menus with search inputs
User->>TypeDropdownSelector: Types in search input (per category)
TypeDropdownSelector->>User: Filters and displays matching types
User->>TypeDropdownSelector: Selects a type
TypeDropdownSelector->>ParentComponent: Calls onSelect(definition, value)
ParentComponent->>ParentComponent: Updates state with selected type
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)src/renderer/components/_atoms/type-dropdown-selector/index.tsx (6)🧬 Code Graph Analysis (1)src/renderer/components/_atoms/type-dropdown-selector/index.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/renderer/components/_molecules/data-types/array/index.tsx (1)
107-112: Remove redundant forEach loops in dimension updates.The
forEachloops are causing redundant calls toupdateDatatype. Each iteration calls the update function with the same data, which is inefficient and could impact performance.-newRows.forEach(() => { - const optionalSchema = { - dimensions: newRows.map((row) => ({ dimension: row?.dimension })), - } - updateDatatype(data.name, optionalSchema as PLCArrayDatatype) -}) +const optionalSchema = { + dimensions: newRows.map((row) => ({ dimension: row?.dimension })), +} +updateDatatype(data.name, optionalSchema as PLCArrayDatatype)Also applies to: 130-136, 153-159
src/renderer/components/_molecules/variables-table/elements/array-modal.tsx (1)
16-54: Centralize type extraction logic to avoid duplication.The type definitions and library extraction logic are duplicated across multiple files. This makes maintenance difficult and increases the risk of inconsistencies.
Consider creating a utility module:
// src/utils/type-selectors.ts export const extractTypeCategories = ( dataTypes: DataType[], libraries: Libraries, currentEditorName?: string ) => { const baseTypes = baseTypeSchema.options.filter(type => type.toUpperCase() !== 'ARRAY') const userDataTypes = dataTypes .map(type => type.name) .filter(name => name !== currentEditorName && name.toUpperCase() !== 'ARRAY') const systemFunctionBlocks = extractSystemFunctionBlocks(libraries.system) const userFunctionBlocks = extractUserFunctionBlocks(libraries.user) return { variableTypes: [ { definition: 'base-type', values: baseTypes }, { definition: 'user-data-type', values: userDataTypes }, ], libraryTypes: [ { definition: 'system', values: systemFunctionBlocks }, { definition: 'user', values: userFunctionBlocks }, ], } }This would eliminate duplication and ensure consistent type categorization across all components.
🧹 Nitpick comments (5)
src/renderer/components/_molecules/data-types/array/index.tsx (1)
15-43: Consider improving type safety for library data structures.The ad-hoc type definitions and complex type assertions in the user library extraction logic could be improved. Consider defining proper types in a shared location or using discriminated unions to handle the different library formats more safely.
Example approach using discriminated unions:
-type Pou = { type: string; name: string } -type UserLibWithPous = { pous: Pou[] } -type UserLibFunctionBlock = { type: string; name: string } +type LibraryPou = { + type: 'function' | 'function-block' | 'program' + name: string +} + +type UserLibrary = + | { type: 'library'; name: string; pous: LibraryPou[] } + | { type: 'function-block'; name: string }Then simplify the extraction logic:
-const userFunctionBlocks = sliceLibraries.user.flatMap((userLib: UserLibWithPous | UserLibFunctionBlock) => - 'pous' in userLib && Array.isArray(userLib.pous) - ? userLib.pous.filter((pou) => pou.type === 'function-block').map((pou) => pou.name.toUpperCase()) - : (userLib as UserLibFunctionBlock).type === 'function-block' - ? [(userLib as UserLibFunctionBlock).name.toUpperCase()] - : [], -) +const userFunctionBlocks = sliceLibraries.user.flatMap((userLib: UserLibrary) => { + if (userLib.type === 'library') { + return userLib.pous + .filter(pou => pou.type === 'function-block') + .map(pou => pou.name.toUpperCase()) + } + return userLib.type === 'function-block' ? [userLib.name.toUpperCase()] : [] +})src/renderer/components/_molecules/data-types/structure/table/selectable-cell.tsx (2)
48-54: Improve readability of nested conditionals.The deeply nested conditionals and type assertions make this code difficult to maintain. Consider extracting the logic into a helper function or using the same type improvements suggested for the array component.
-'pous' in userLibrary && Array.isArray((userLibrary as { pous: { type: string; name: string }[] }).pous) - ? (userLibrary as { pous: { type: string; name: string }[] }).pous - .filter((pou) => pou.type === 'function-block') - .map((pou) => pou.name.toUpperCase()) - : userLibrary.type === 'function-block' - ? [userLibrary.name.toUpperCase()] - : [], +const extractFunctionBlocks = (lib: UserLibrary): string[] => { + if ('pous' in lib && Array.isArray(lib.pous)) { + return lib.pous + .filter(pou => pou.type === 'function-block') + .map(pou => pou.name.toUpperCase()) + } + return lib.type === 'function-block' ? [lib.name.toUpperCase()] : [] +}
120-220: Reduce code duplication in dropdown rendering.The dropdown structure for variable types and library types is nearly identical. Consider extracting a reusable component or helper to reduce duplication.
Create a reusable dropdown section:
const TypeDropdownSection = ({ scopes, filters, onFilterChange, onSelect, filterKey = 'default' }: { scopes: Array<{ definition: string; values: string[] }> filters: Record<string, string> | string onFilterChange: (value: string, key?: string) => void onSelect: (definition: string, value: string) => void filterKey?: string }) => ( <> {scopes.map((scope) => { const filterValue = typeof filters === 'string' ? filters : filters[scope.definition] return ( <PrimitiveDropdown.Sub key={scope.definition} onOpenChange={() => onFilterChange('', scope.definition)} > {/* Dropdown content */} </PrimitiveDropdown.Sub> ) })} </> )Then use it for both sections:
-{filteredVariableValues.map((scope) => ( - // ... duplicate dropdown structure -))} -{filteredLibraryValues.map((scope) => ( - // ... duplicate dropdown structure -))} +<TypeDropdownSection + scopes={filteredVariableValues} + filters={variableFilters} + onFilterChange={(value, key) => setVariableFilters(prev => ({ ...prev, [key]: value }))} + onSelect={(def, val) => onSelect(def as PLCStructureVariable['type']['definition'], val)} +/> +<TypeDropdownSection + scopes={filteredLibraryValues} + filters={inputFilter} + onFilterChange={setInputFilter} + onSelect={(_, val) => onSelect('derived', val)} +/>src/renderer/components/_molecules/variables-table/selectable-cell.tsx (1)
241-362: Consider extracting common dropdown filtering logic.This component has the same dropdown structure duplication as the structure table's selectable cell. Consider creating a shared component or hook that both files can use to reduce code duplication across the codebase.
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (1)
24-24: Fix typo in state variable name.There's a typo in the state variable name that should be corrected for consistency and readability.
- const [poppoverIsOpen, setPoppoverIsOpen] = useState(false) + const [popoverIsOpen, setPopoverIsOpen] = useState(false)And update the usage in line 42:
- <PrimitiveDropdown.Root onOpenChange={setPoppoverIsOpen} open={poppoverIsOpen}> + <PrimitiveDropdown.Root onOpenChange={setPopoverIsOpen} open={popoverIsOpen}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/renderer/components/_atoms/dimensions-modal/index.tsx(5 hunks)src/renderer/components/_atoms/type-dropdown-selector/index.tsx(1 hunks)src/renderer/components/_molecules/data-types/array/index.tsx(4 hunks)src/renderer/components/_molecules/data-types/structure/table/elements/array-modal.tsx(3 hunks)src/renderer/components/_molecules/data-types/structure/table/selectable-cell.tsx(5 hunks)src/renderer/components/_molecules/variables-table/elements/array-modal.tsx(3 hunks)src/renderer/components/_molecules/variables-table/selectable-cell.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
src/renderer/components/_atoms/dimensions-modal/index.tsx (4)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#251
File: src/renderer/components/_atoms/dimensions-modal/index.tsx:1-1
Timestamp: 2025-05-23T14:11:13.511Z
Learning: The Radix UI library in this codebase supports named exports such as `import { ScrollArea } from '@radix-ui/react-scroll-area'` rather than requiring namespace imports like `import * as ScrollArea`.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#274
File: src/types/PLC/devices/configuration.ts:0-0
Timestamp: 2025-07-04T15:13:57.599Z
Learning: In src/types/PLC/devices/configuration.ts, the team prefers to handle IP address validation at the local store level when passing values from frontend to backend, rather than adding additional validation at the schema level.
src/renderer/components/_molecules/data-types/array/index.tsx (6)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#250
File: src/utils/PLC/xml-generator/old-editor/data-type-xml.ts:21-26
Timestamp: 2025-06-04T18:43:50.030Z
Learning: In src/utils/PLC/xml-generator/old-editor/data-type-xml.ts, the team prefers to keep complex nested ternary operators for type determination rather than refactoring them into helper functions, to avoid introducing additional functions.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#251
File: src/renderer/components/_atoms/dimensions-modal/index.tsx:1-1
Timestamp: 2025-05-23T14:11:13.511Z
Learning: The Radix UI library in this codebase supports named exports such as `import { ScrollArea } from '@radix-ui/react-scroll-area'` rather than requiring namespace imports like `import * as ScrollArea`.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:126-129
Timestamp: 2025-04-10T18:27:32.877Z
Learning: In the OpenPLC Editor project, the team avoids using TypeScript's `any` type even when dealing with dynamic data structures.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#274
File: src/types/PLC/devices/configuration.ts:0-0
Timestamp: 2025-07-04T15:13:57.599Z
Learning: In src/types/PLC/devices/configuration.ts, the team prefers to handle IP address validation at the local store level when passing values from frontend to backend, rather than adding additional validation at the schema level.
src/renderer/components/_molecules/variables-table/elements/array-modal.tsx (5)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#250
File: src/utils/PLC/xml-generator/old-editor/data-type-xml.ts:21-26
Timestamp: 2025-06-04T18:43:50.030Z
Learning: In src/utils/PLC/xml-generator/old-editor/data-type-xml.ts, the team prefers to keep complex nested ternary operators for type determination rather than refactoring them into helper functions, to avoid introducing additional functions.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:126-129
Timestamp: 2025-04-10T18:27:32.877Z
Learning: In the OpenPLC Editor project, the team avoids using TypeScript's `any` type even when dealing with dynamic data structures.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#274
File: src/types/PLC/devices/configuration.ts:0-0
Timestamp: 2025-07-04T15:13:57.599Z
Learning: In src/types/PLC/devices/configuration.ts, the team prefers to handle IP address validation at the local store level when passing values from frontend to backend, rather than adding additional validation at the schema level.
src/renderer/components/_molecules/variables-table/selectable-cell.tsx (5)
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:380-407
Timestamp: 2025-04-10T18:29:20.310Z
Learning: In the BlockNodeElement component's useEffect for handling variable input focus and initialization, the effect should only run when the data prop changes, not when other referenced variables change. This is an intentional design decision.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#250
File: src/utils/PLC/xml-generator/old-editor/data-type-xml.ts:21-26
Timestamp: 2025-06-04T18:43:50.030Z
Learning: In src/utils/PLC/xml-generator/old-editor/data-type-xml.ts, the team prefers to keep complex nested ternary operators for type determination rather than refactoring them into helper functions, to avoid introducing additional functions.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:126-129
Timestamp: 2025-04-10T18:27:32.877Z
Learning: In the OpenPLC Editor project, the team avoids using TypeScript's `any` type even when dealing with dynamic data structures.
src/renderer/components/_molecules/data-types/structure/table/selectable-cell.tsx (4)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#251
File: src/renderer/components/_atoms/dimensions-modal/index.tsx:1-1
Timestamp: 2025-05-23T14:11:13.511Z
Learning: The Radix UI library in this codebase supports named exports such as `import { ScrollArea } from '@radix-ui/react-scroll-area'` rather than requiring namespace imports like `import * as ScrollArea`.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:126-129
Timestamp: 2025-04-10T18:27:32.877Z
Learning: In the OpenPLC Editor project, the team avoids using TypeScript's `any` type even when dealing with dynamic data structures.
src/renderer/components/_molecules/data-types/structure/table/elements/array-modal.tsx (5)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#250
File: src/utils/PLC/xml-generator/old-editor/data-type-xml.ts:21-26
Timestamp: 2025-06-04T18:43:50.030Z
Learning: In src/utils/PLC/xml-generator/old-editor/data-type-xml.ts, the team prefers to keep complex nested ternary operators for type determination rather than refactoring them into helper functions, to avoid introducing additional functions.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#223
File: src/renderer/components/_atoms/graphical-editor/fbd/block.tsx:126-129
Timestamp: 2025-04-10T18:27:32.877Z
Learning: In the OpenPLC Editor project, the team avoids using TypeScript's `any` type even when dealing with dynamic data structures.
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#274
File: src/types/PLC/devices/configuration.ts:0-0
Timestamp: 2025-07-04T15:13:57.599Z
Learning: In src/types/PLC/devices/configuration.ts, the team prefers to handle IP address validation at the local store level when passing values from frontend to backend, rather than adding additional validation at the schema level.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (3)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#251
File: src/renderer/components/_atoms/dimensions-modal/index.tsx:1-1
Timestamp: 2025-05-23T14:11:13.511Z
Learning: The Radix UI library in this codebase supports named exports such as `import { ScrollArea } from '@radix-ui/react-scroll-area'` rather than requiring namespace imports like `import * as ScrollArea`.
🧬 Code Graph Analysis (4)
src/renderer/components/_molecules/data-types/array/index.tsx (1)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (1)
TypeDropdownSelector(16-173)
src/renderer/components/_molecules/variables-table/elements/array-modal.tsx (3)
src/renderer/store/index.ts (1)
useOpenPLCStore(64-64)src/shared/data/common.ts (1)
baseTypes(35-35)src/types/PLC/open-plc.ts (1)
baseTypeSchema(290-290)
src/renderer/components/_molecules/variables-table/selectable-cell.tsx (1)
src/renderer/components/_atoms/index.ts (1)
InputWithRef(8-8)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (1)
src/renderer/components/_atoms/index.ts (1)
InputWithRef(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: test (windows-latest)
🔇 Additional comments (8)
src/renderer/components/_atoms/dimensions-modal/index.tsx (1)
77-82: Clean integration of TypeDropdownSelector.The refactoring properly replaces the Select component with TypeDropdownSelector and correctly handles the new API by ignoring the definition parameter since only the value is needed here.
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (3)
26-29: Good design choice for separate filtering approaches.The implementation correctly uses individual filters for variable types (allowing independent search per category) while sharing a single filter for library types. This provides a good balance between functionality and simplicity.
89-89: Proper event propagation handling.Good use of
stopPropagation()to prevent the dropdown from closing when typing in the search input fields.
96-96: No runtime validation needed for definition castThe
scope.definitionpassed into this component is already constrained by your upstream Zod schemas to only'base-type'or'user-data-type'(seesrc/types/PLC/units/variable.tsand howvariableTypesis built insrc/utils/generate-iec-string-to-variables.ts). As such:
variableTypesnever contains a'derived'definition here- The cast to
as 'base-type' | 'user-data-type'is safe- Your
onSelectprop signature allows'derived'for other contexts but isn’t used in this dropdownNo additional runtime checks are required. If you prefer stricter typing, you could narrow the
onSelectparameter to exclude'derived', but it’s not necessary for correctness.src/renderer/components/_molecules/data-types/structure/table/elements/array-modal.tsx (4)
16-18: Well-defined types for library data structures.The type definitions appropriately handle the different shapes of library data, enabling proper type safety for the extraction logic below.
47-53: Robust handling of heterogeneous user library structures.The type guard logic effectively handles different user library shapes (arrays vs single objects) while maintaining type safety. The nested conditionals properly extract function blocks from both structures.
37-42: Consistent filtering logic for array base types.Good implementation that filters out 'ARRAY' from both base types and user data types, and excludes the current type name to prevent recursive references.
204-205: DimensionsModal prop compatibility verified
TheDimensionsModalPropsinterface now declares bothvariableTypesandlibraryTypesand no longer includesallTypes. No further updates are needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (2)
Learnt from: vmleroy
PR: Autonomy-Logic/openplc-editor#265
File: src/renderer/components/_atoms/graphical-editor/ladder/block.tsx:149-153
Timestamp: 2025-06-16T14:31:51.786Z
Learning: In src/renderer/components/_atoms/graphical-editor/ladder/block.tsx, the libraries.system data is dynamic and imported from JSON with varying structure, making proper static typing infeasible. The @ts-expect-error and eslint-disable comments are justified when dynamically probing the structure at runtime.
Learnt from: JoaoGSP
PR: Autonomy-Logic/openplc-editor#218
File: src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx:73-73
Timestamp: 2025-03-14T14:46:03.671Z
Learning: Empty useEffect hooks in the VariableElement component (src/renderer/components/_atoms/graphical-editor/fbd/variable.tsx) are intentional placeholders that will be implemented later as they depend on other logic.
🧬 Code Graph Analysis (1)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (1)
src/renderer/components/_atoms/index.ts (1)
InputWithRef(8-8)
🔇 Additional comments (3)
src/renderer/components/_atoms/type-dropdown-selector/index.tsx (3)
29-36: LGTM! Well-structured dropdown with proper accessibility and styling.The dropdown trigger implementation is solid with proper disabled state handling, responsive styling, and dark mode support.
37-42: LGTM! Proper portal implementation with good styling.The portal and content structure follows Radix UI best practices with appropriate positioning and styling.
157-161: LGTM! Proper component structure completion.The component is properly closed with correct Radix UI component hierarchy.
JulioSergioFS
requested changes
Jul 17, 2025
Contributor
JulioSergioFS
left a comment
JulioSergioFS
left a comment
There was a problem hiding this comment.
Review coderabbit suggestions and mine to be approved.
thiagoralves
approved these changes
Jul 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request info
Link to Jira task
jira task DOPE-60
jira task DOPE-61
Description of the changes proposed
DOD checklist
Summary by CodeRabbit
New Features
Refactor