Repository navigation
fix(expressions): carry extraActions through the shell and operand rows - #118
Merged
Merged
Conversation
The builder's extraActions seam is where a host re-attaches per-card actions it owns — the IDE's AI-assist button. It stopped one hop short of the component hosts actually mount: ExpressionField neither declared nor forwarded it, TemplateField's expression mode handed it to neither editor, and the builder's two operand TemplateField mounts passed nothing host-injected. The group recursion already forwarded it, which is why the builder's own seam story stayed green while an operand that is itself an expression silently lost the action — the regression qorus-ide hit the moment it adopted the shell. Forward it through each hop as an explicit typed prop, in the repo's existing style: declared on ExpressionField and TemplateField (and destructured there, so it never rides `rest` onto a leaf input), passed into both operand mounts. The picker modal gains a library-owned class so hosts' tests have something to target — qorus-ide's helpers looked for a class this modal never carried. A new ExpressionField story pins the two hops with the IDE's own button presentation injected and the assertion scoped to the nested card. design/IDE_INTEGRATION.md gets dated revision notes: the seam table now names the shell, and its reference to a task file that no longer exists is gone. Version 0.10.51, one patch over the published 0.10.50. Closes #116
The task workflow appends the landed sha to the task's status and the dashboard row once a batch commits; f5fb2ee is that commit.
|
📦 Prerelease published for this PR: Install it in a consumer (updates yarn add @qoretechnologies/reqraft@0.10.51-pr.118.g067625ePublished under the |
qlip visual regression🔍 6 changed, 1 new of 470 snapshots. |
…ares The rest-argument operand mount derived an empty argument's `type` and `defaultType` from its value, which is undefined when there is no value. So an empty `bool` argument such as String Contains' "Ignore case?" rendered as an untyped `auto` field — a type picker sitting under a label that already said "true or false" — and an empty `any` argument never qualified for TemplateField's template-selector default, because that rule reads `type` and `type` was undefined. The first-argument mount and the allowed-values branch already fell back to the declared type; this makes the rest-argument mount do the same. Inherited from the IDE source the builder was ported from; the IDE picks the fix up through reqraft. Flagged by Foxhoundn on the qlip review of #118. Recorded as a dated departure from the verbatim port in EXPRESSION_BUILDER_REPORT_STRATEGY.md.
nickmazurenko
added a commit
that referenced
this pull request
Sep 8, 2026
develop is at 0.10.51 (PR #118), so this PR's single bump lands on 0.10.52.
davidnich
added a commit
that referenced
this pull request
Sep 8, 2026
`fix/per-field-templates` and #118 both threaded a host-supplied prop through the same three files, so `ExpressionField` and the expression builder conflicted on adjacent lines. Both props are wanted at every site and the resolution is additive throughout: `componentOverrides` (the host's per-`ui_type` editors) and `extraActions` (the host's per-card actions) travel the same path for different payloads. While resolving, `componentOverrides` adopts the convention #118 established for `extraActions` in `TemplateField`: declare it on the props interface and destructure it, rather than reading it back out of `rest`. The old form left it in `rest`, which is spread onto `ReqoreControlGroup`, `ReqoreMenuSection` and `LongStringField` — a hash of React components handed to layout components that forward unknown props to the DOM. It is now passed explicitly to the two places that render a field component and nowhere else, which is what #118's comment warns to do. Typecheck clean; 79 files / 1076 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K9Lq4BrEaa1sAj8cMRWZ9t
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
1 participant
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.
Closes #116.
Why
The builder's
extraActionsseam is where a host re-attaches per-card actions it owns — the IDE's AI-assist button. It stopped one hop short of the component hosts actually mount:ExpressionFieldneither declared nor forwarded it,TemplateField's expression mode handed it to neither editor, and the builder's two operandTemplateFieldmounts passed nothing host-injected. The group recursion already forwarded it, which is why the builder's own seam story stayed green while an operand that is itself an expression silently lost the action — the regression qorus-ide hit the moment it adopted the shell (qorus-ide#275).What changed
ExpressionField: declaresextraActionsand forwards it to the builder.TemplateField: declares it, destructures it (so it never ridesrestonto a leaf input), forwards it to both editors its expression mode renders.builder/index.tsx: passes it into both operandTemplateFieldmounts, matching what the group recursion already did.SelectCollection: the picker modal carries a library-ownedreqraft-select-dialogclass so hosts' tests have something to target.ExpressionField › NestedOperandKeepsInjectedActions: the IDE's own button presentation injected, assertion scoped to the nested card plus a total count of two.design/IDE_INTEGRATION.md: dated revision notes — the seam table now names the shell, and a reference to a task file that no longer exists is gone. Task file + index row per the repo workflow.0.10.50→0.10.51(one patch over the published 0.10.50).Verification
build:test:prod,lint, unit (57 files / 919 tests) green; pre-push hook ran them again on push.test:stories ExpressionField8/8; the new story's local qlip capture read — the injected button renders in both card headers.Out of scope, tracked in #116
Saved values at operand level (the builder owns operand
menuItemsfor its type picker, so that needs an additive seam; the feature is currently off in every IDE surface), translations, tour registration, the guided server-expression flow, dropdown search copy, the picker's action-name badge — none has a designed seam yet.Note for reviewers
expressionsUrlis honoured by the shell (it resolves it in its ownuseExpressionsand hands the merged catalogue down); an earlier draft of the issue said otherwise and was corrected.