fix(pou): show return type selector for graphical function POUs#954
Conversation
The return type selector in the variables editor was gated on editor.type === 'plc-textual', so functions written in FBD, LD, or SFC had no way to select their return type even though the store action, POU creation defaults, and the ST transpiler already fully support return types for graphical functions. Extend the condition to include 'plc-graphical'. Add a component test covering the selector's visibility for FBD/LD/ST functions and its absence for programs and function blocks. Fixes Autonomy-Logic#696 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe VariablesEditor now shows the “Return type” control for function POUs in graphical editors. Tests cover visibility across supported languages and confirm it remains hidden for programs and function blocks. ChangesReturn type selector visibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/frontend/components/_organisms/variables-editor/__tests__/return-type-visibility.test.tsx`:
- Around line 11-14: Extend the createPou helper and its language type to accept
sfc, then add a separate SFC fixture/test covering return-type visibility
alongside the existing FBD and LD cases. Preserve the ST test independently
rather than replacing it, so both textual and SFC coverage remain.
In `@src/frontend/components/_organisms/variables-editor/index.tsx`:
- Line 1055: Update the return-type selector rendered by the
plc-textual/plc-graphical function branch to use a dedicated matching ID, such
as return-type, on its SelectTrigger and associated label htmlFor; preserve the
existing class-filter ID for the class filter, and add a test assertion
verifying the return-type control is correctly associated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9342d035-fa1a-414d-ab53-0cdf1ea9a2a5
📒 Files selected for processing (2)
src/frontend/components/_organisms/variables-editor/__tests__/return-type-visibility.test.tsxsrc/frontend/components/_organisms/variables-editor/index.tsx
|
Verified the fix end-to-end in the running app (production build, Playwright driving the real Electron window): created a project, created a Function POU with FBD language, and confirmed the Return type selector visible for the FBD function (the field this issue reports missing): Dropdown opens with the full IEC type list: Selection persists (INT): |
Address review feedback: the return type SelectTrigger reused the
class filter's id ("class-filter"), producing duplicate ids when both
render, and the label's htmlFor pointed at nothing. Use a dedicated
"return-type" id wired to the label, assert the association in a test,
and add an SFC function case alongside FBD/LD/ST.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
1 participant
Pull request info
References
This PR resolves #696.
Link to Jira task
External contribution — no Jira access; branch follows the
bugfix/gh-<n>-<slug>convention from CONTRIBUTING.md.Description of the changes proposed
editor.type === 'plc-textual', so functions written in FBD, LD, or SFC had no way to select their return type (the field simply never appeared, as reported in Function Definition - Return Type Cannot Be Selected in FDB #696).projectActions.updatePouReturnTypeis language-agnostic,createPouObjectdefaults new functions toBOOL, and the ST transpiler emitsFUNCTION <name> : <returnType>for graphical POUs (emit/pou-graphical.ts). The selector's render condition was the only gap.(editor.type === 'plc-textual' || editor.type === 'plc-graphical') && editor.meta.pouType === 'function'. Both editor model variants carry the samemeta.pouTypeshape, so TypeScript narrowing is unaffected.return-type-visibility.test.tsx, 5 cases): selector visible for FBD, LD, and ST functions; absent for an FBD program and an FBD function block. The FBD and LD cases fail without the fix and pass with it.DOD checklist
board-info-resolverfailures on Windows are unrelated and fail on a clean checkout too).🤖 Generated with Claude Code
https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
Summary by CodeRabbit
New Features
Bug Fixes
Tests