Skip to content

Commit 76feac2

Browse files
mmelkoclaude
authored andcommitted
fix(DataMapper): skip rendering nested choice node if both are selected (#3183)
Problem: When xs:choice is nested, the intermediate choice wrapper node stays rendered even when both parent and child choices are selected. Changes: - Recurse through nested selected wrappers in doGenerateNodeDataFromWrapperField to flatten the tree and render the final selected member directly - Resolve outermost selected wrapper in context menu so "Show All Choice Options" fully reverts the entire nested selection chain - Add resolveOutermostSelectedWrapper helper and getSelectedChoiceDepth to VisualizationUtilService - Render selected choice as a green pill Label with Choices icon; show ×N depth multiplier when multiple wrapper levels are collapsed - Add tests for both-selected and outer-only-selected nested choice scenarios Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent effb1a4 commit 76feac2

6 files changed

Lines changed: 184 additions & 45 deletions

File tree

packages/ui/src/components/Document/Nodes/BaseNode.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import './BaseNode.scss';
22

33
import { At, ChevronDown, ChevronRight, Choices, DocumentComment, Draggable, ValueVariable } from '@carbon/icons-react';
4-
import { Button, Icon, Tooltip } from '@patternfly/react-core';
4+
import { Button, Icon, Label, Tooltip } from '@patternfly/react-core';
55
import { LayerGroupIcon } from '@patternfly/react-icons';
66
import { FunctionComponent, MouseEventHandler, PropsWithChildren, ReactNode, useCallback, useState } from 'react';
77

@@ -67,6 +67,7 @@ export const BaseNode: FunctionComponent<PropsWithChildren<BaseNodeProps>> = ({
6767
const isCollectionField = VisualizationUtilService.isCollectionField(nodeData);
6868
const isChoiceField = VisualizationUtilService.isChoiceField(nodeData);
6969
const isSelectedChoice = VisualizationUtilService.isSelectedChoiceField(nodeData);
70+
const choiceDepth = VisualizationUtilService.getSelectedChoiceDepth(nodeData);
7071
const isAbstractField = VisualizationUtilService.isAbstractField(nodeData);
7172
const isSelectedAbstract = VisualizationUtilService.isSelectedAbstractField(nodeData);
7273
const isAttributeField = VisualizationUtilService.isAttributeField(nodeData);
@@ -126,15 +127,16 @@ export const BaseNode: FunctionComponent<PropsWithChildren<BaseNodeProps>> = ({
126127
<LayerGroupIcon />
127128
</Icon>
128129
)}
129-
{isChoiceField && (
130-
<Icon
131-
className="node__spacer"
132-
status={isSelectedChoice ? 'success' : undefined}
133-
data-testid="choice-field-icon"
134-
>
130+
{isChoiceField && !isSelectedChoice && (
131+
<Icon className="node__spacer" data-testid="choice-field-icon">
135132
<Choices />
136133
</Icon>
137134
)}
135+
{isChoiceField && isSelectedChoice && (
136+
<Label className="node__spacer" color="green" icon={<Choices />} isCompact data-testid="choice-field-icon">
137+
{choiceDepth >= 2 ? <>&times;{choiceDepth}</> : ''}
138+
</Label>
139+
)}
138140
{isAbstractField && (
139141
<Icon
140142
className="node__spacer"

packages/ui/src/components/Document/actions/FieldContextMenu/useChoiceContextMenu.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ function resolveChoiceNodeInfo(nodeData: NodeData): ChoiceNodeInfo {
6161
: undefined;
6262

6363
let choiceWrapperField: IField | undefined;
64-
if (isChoiceWrapper) {
64+
if (isSelectedChoice) {
65+
choiceWrapperField = VisualizationUtilService.resolveOutermostSelectedWrapper(nodeData.choiceField).outermost;
66+
} else if (isChoiceWrapper) {
6567
choiceWrapperField = field;
66-
} else if (isSelectedChoice) {
67-
choiceWrapperField = nodeData.choiceField;
6868
}
6969

7070
return {

packages/ui/src/services/visualization/visualization-util.service.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ import { TestUtil } from '../../stubs/datamapper/data-mapper';
1414
import { XmlSchemaDocument } from '../document/xml-schema/xml-schema-document.model';
1515
import { VisualizationUtilService } from './visualization-util.service';
1616

17+
function createMockField(baseField: IField, overrides: Partial<IField> = {}): IField {
18+
return { ...baseField, ...overrides };
19+
}
20+
1721
describe('VisualizationUtilService', () => {
1822
let sourceDoc: XmlSchemaDocument;
1923
let sourceDocNode: DocumentNodeData;
@@ -186,4 +190,109 @@ describe('VisualizationUtilService', () => {
186190
expect(VisualizationUtilService.getField(sourceDocNode)).toBeUndefined();
187191
});
188192
});
193+
194+
describe('resolveOutermostSelectedWrapper', () => {
195+
it('should return the field itself and depth 1 when no parent wrapper', () => {
196+
const field = createMockField(sourceDoc.fields[0], { wrapperKind: 'choice', selectedMemberIndex: 0 });
197+
const result = VisualizationUtilService.resolveOutermostSelectedWrapper(field);
198+
expect(result.outermost).toBe(field);
199+
expect(result.depth).toEqual(1);
200+
});
201+
202+
it('should walk up through selected parent wrappers', () => {
203+
const outer = createMockField(sourceDoc.fields[0], { wrapperKind: 'choice', selectedMemberIndex: 0 });
204+
const inner = createMockField(sourceDoc.fields[0], {
205+
wrapperKind: 'choice',
206+
selectedMemberIndex: 1,
207+
parent: outer,
208+
});
209+
const result = VisualizationUtilService.resolveOutermostSelectedWrapper(inner);
210+
expect(result.outermost).toBe(outer);
211+
expect(result.depth).toEqual(2);
212+
});
213+
214+
it('should walk up through three levels of selected wrappers', () => {
215+
const outermost = createMockField(sourceDoc.fields[0], { wrapperKind: 'choice', selectedMemberIndex: 0 });
216+
const middle = createMockField(sourceDoc.fields[0], {
217+
wrapperKind: 'choice',
218+
selectedMemberIndex: 0,
219+
parent: outermost,
220+
});
221+
const inner = createMockField(sourceDoc.fields[0], {
222+
wrapperKind: 'choice',
223+
selectedMemberIndex: 1,
224+
parent: middle,
225+
});
226+
const result = VisualizationUtilService.resolveOutermostSelectedWrapper(inner);
227+
expect(result.outermost).toBe(outermost);
228+
expect(result.depth).toEqual(3);
229+
});
230+
231+
it('should stop at parent without selectedMemberIndex', () => {
232+
const outer = createMockField(sourceDoc.fields[0], { wrapperKind: 'choice' });
233+
const inner = createMockField(sourceDoc.fields[0], {
234+
wrapperKind: 'choice',
235+
selectedMemberIndex: 0,
236+
parent: outer,
237+
});
238+
const result = VisualizationUtilService.resolveOutermostSelectedWrapper(inner);
239+
expect(result.outermost).toBe(inner);
240+
expect(result.depth).toEqual(1);
241+
});
242+
243+
it('should return depth 1 and undefined for undefined input', () => {
244+
const result = VisualizationUtilService.resolveOutermostSelectedWrapper(undefined);
245+
expect(result.outermost).toBeUndefined();
246+
expect(result.depth).toEqual(1);
247+
});
248+
});
249+
250+
describe('getSelectedChoiceDepth', () => {
251+
it('should return 0 for non-choice nodes', () => {
252+
const fieldNode = new FieldNodeData(sourceDocNode, sourceDoc.fields[0]);
253+
expect(VisualizationUtilService.getSelectedChoiceDepth(fieldNode)).toEqual(0);
254+
});
255+
256+
it('should return 0 for unselected choice nodes', () => {
257+
const choiceField = {
258+
...sourceDoc.fields[0],
259+
wrapperKind: 'choice' as const,
260+
fields: [],
261+
} as unknown as IField;
262+
const choiceNode = new ChoiceFieldNodeData(sourceDocNode, choiceField);
263+
expect(VisualizationUtilService.getSelectedChoiceDepth(choiceNode)).toEqual(0);
264+
});
265+
266+
it('should return 1 for simple selected choice', () => {
267+
const wrapper = {
268+
...sourceDoc.fields[0],
269+
wrapperKind: 'choice' as const,
270+
selectedMemberIndex: 0,
271+
fields: [{ ...sourceDoc.fields[0], name: 'a', fields: [] }],
272+
} as unknown as IField;
273+
const choiceNode = new ChoiceFieldNodeData(sourceDocNode, sourceDoc.fields[0]);
274+
choiceNode.choiceField = wrapper;
275+
expect(VisualizationUtilService.getSelectedChoiceDepth(choiceNode)).toEqual(1);
276+
});
277+
278+
it('should return 3 for triple-nested selected choice', () => {
279+
const outermost = createMockField(sourceDoc.fields[0], {
280+
wrapperKind: 'choice',
281+
selectedMemberIndex: 0,
282+
});
283+
const middle = createMockField(sourceDoc.fields[0], {
284+
wrapperKind: 'choice',
285+
selectedMemberIndex: 0,
286+
parent: outermost,
287+
});
288+
const inner = createMockField(sourceDoc.fields[0], {
289+
wrapperKind: 'choice',
290+
selectedMemberIndex: 0,
291+
parent: middle,
292+
});
293+
const choiceNode = new ChoiceFieldNodeData(sourceDocNode, sourceDoc.fields[0]);
294+
choiceNode.choiceField = inner;
295+
expect(VisualizationUtilService.getSelectedChoiceDepth(choiceNode)).toEqual(3);
296+
});
297+
});
189298
});

packages/ui/src/services/visualization/visualization-util.service.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,39 @@ export class VisualizationUtilService {
8989
return VisualizationUtilService.isSelectedChoiceField(nodeData) && nodeData.field.wrapperKind === 'choice';
9090
}
9191

92+
/**
93+
* Walks up from `wrapper` through ancestor choice fields that also have a selection,
94+
* returning the outermost selected wrapper and the number of levels traversed.
95+
*/
96+
static resolveOutermostSelectedWrapper(wrapper: IField | undefined): {
97+
outermost: IField | undefined;
98+
depth: number;
99+
} {
100+
let depth = 1;
101+
let current = wrapper;
102+
while (
103+
current?.parent &&
104+
'wrapperKind' in current.parent &&
105+
current.parent.wrapperKind === 'choice' &&
106+
current.parent.selectedMemberIndex !== undefined
107+
) {
108+
depth++;
109+
current = current.parent;
110+
}
111+
return { outermost: current, depth };
112+
}
113+
114+
/**
115+
* Returns the number of resolved choice wrapper levels for a selected choice node.
116+
* A simple selected choice returns 1. A flattened nested choice (both outer and inner
117+
* selected) returns 2+, indicating how many wrapper levels were collapsed.
118+
* Returns 0 for non-choice nodes.
119+
*/
120+
static getSelectedChoiceDepth(nodeData: NodeData): number {
121+
if (!VisualizationUtilService.isSelectedChoiceField(nodeData)) return 0;
122+
return VisualizationUtilService.resolveOutermostSelectedWrapper(nodeData.choiceField).depth;
123+
}
124+
92125
/**
93126
* Returns `true` if the node is an abstract field, on either source or target side.
94127
* @param nodeData - The node to test.

packages/ui/src/services/visualization/visualization.service.choice.test.ts

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -454,65 +454,54 @@ describe('VisualizationService / choice fields', () => {
454454
expect(outerChildren[1].title).toEqual('regularField');
455455
});
456456

457-
it('both selected: resolves in two steps with choiceField references at each level', () => {
458-
const { outerChoice, innerChoice } = createNestedChoiceFields(0, 1);
457+
it('both selected: flattens nested wrappers and shows final selected member directly', () => {
458+
const { outerChoice, innerChoice, innerMembers } = createNestedChoiceFields(0, 1);
459459
const parentField = { ...sourceDoc.fields[0], fields: [outerChoice] };
460460
const parentNode = new FieldNodeData(sourceDocNode, parentField as (typeof sourceDoc.fields)[0]);
461461

462462
const children = VisualizationService.generateNonDocumentNodeDataChildren(parentNode);
463463
expect(children.length).toEqual(1);
464-
const outerNode = children[0] as ChoiceFieldNodeData;
465-
expect(outerNode).toBeInstanceOf(ChoiceFieldNodeData);
466-
expect(outerNode.field).toBe(innerChoice);
467-
expect(outerNode.choiceField).toBe(outerChoice);
468-
469-
const innerChildren = VisualizationService.generateNonDocumentNodeDataChildren(outerNode);
470-
expect(innerChildren.length).toEqual(1);
471-
const innerNode = innerChildren[0] as ChoiceFieldNodeData;
472-
expect(innerNode).toBeInstanceOf(ChoiceFieldNodeData);
473-
expect(innerNode.title).toEqual('y');
474-
expect(innerNode.choiceField).toBe(innerChoice);
464+
const flattenedNode = children[0] as ChoiceFieldNodeData;
465+
expect(flattenedNode).toBeInstanceOf(ChoiceFieldNodeData);
466+
expect(flattenedNode.field).toBe(innerMembers[1]);
467+
expect(flattenedNode.field.name).toEqual('y');
468+
expect(flattenedNode.choiceField).toBe(innerChoice);
475469
});
476470

477-
it('multi-step revert: reverting inner selection restores inner choice members', () => {
471+
it('multi-step revert: reverting inner selection shows inner choice as unselected wrapper', () => {
478472
const { outerChoice, innerChoice } = createNestedChoiceFields(0, 1);
479473
const parentField = { ...sourceDoc.fields[0], fields: [outerChoice] };
480474
const parentNode = new FieldNodeData(sourceDocNode, parentField as (typeof sourceDoc.fields)[0]);
481475

482-
const children = VisualizationService.generateNonDocumentNodeDataChildren(parentNode);
483-
const outerNode = children[0] as ChoiceFieldNodeData;
484-
const innerChildren = VisualizationService.generateNonDocumentNodeDataChildren(outerNode);
485-
const innerNode = innerChildren[0] as ChoiceFieldNodeData;
486-
expect(innerNode.choiceField).toBe(innerChoice);
476+
innerChoice.selectedMemberIndex = undefined;
487477

488-
innerNode.choiceField!.selectedMemberIndex = undefined;
478+
const children = VisualizationService.generateNonDocumentNodeDataChildren(parentNode);
479+
expect(children.length).toEqual(1);
480+
const node = children[0] as ChoiceFieldNodeData;
481+
expect(node).toBeInstanceOf(ChoiceFieldNodeData);
482+
expect(node.field).toBe(innerChoice);
483+
expect(node.choiceField).toBe(outerChoice);
489484

490-
const refreshedInnerChildren = VisualizationService.generateNonDocumentNodeDataChildren(outerNode);
491-
expect(refreshedInnerChildren.length).toEqual(2);
492-
expect(refreshedInnerChildren[0].title).toEqual('x');
493-
expect(refreshedInnerChildren[1].title).toEqual('y');
494-
expect((refreshedInnerChildren[0] as ChoiceFieldNodeData).choiceField).toBeUndefined();
485+
const innerChildren = VisualizationService.generateNonDocumentNodeDataChildren(node);
486+
expect(innerChildren.length).toEqual(2);
487+
expect(innerChildren[0].title).toEqual('x');
488+
expect(innerChildren[1].title).toEqual('y');
495489
});
496490

497-
it('multi-step revert: reverting outer selection after inner restores outer choice members', () => {
491+
it('multi-step revert: reverting outer selection restores outer choice members', () => {
498492
const { outerChoice, innerChoice } = createNestedChoiceFields(0, 1);
499493
const parentField = { ...sourceDoc.fields[0], fields: [outerChoice] };
500494
const parentNode = new FieldNodeData(sourceDocNode, parentField as (typeof sourceDoc.fields)[0]);
501495

502496
innerChoice.selectedMemberIndex = undefined;
497+
outerChoice.selectedMemberIndex = undefined;
503498

504499
const children = VisualizationService.generateNonDocumentNodeDataChildren(parentNode);
500+
expect(children.length).toEqual(1);
505501
const outerNode = children[0] as ChoiceFieldNodeData;
506-
expect(outerNode.choiceField).toBe(outerChoice);
507-
508-
outerNode.choiceField!.selectedMemberIndex = undefined;
509-
510-
const refreshedChildren = VisualizationService.generateNonDocumentNodeDataChildren(parentNode);
511-
expect(refreshedChildren.length).toEqual(1);
512-
const refreshedOuterNode = refreshedChildren[0] as ChoiceFieldNodeData;
513-
expect(refreshedOuterNode.choiceField).toBeUndefined();
502+
expect(outerNode.choiceField).toBeUndefined();
514503

515-
const outerMembers = VisualizationService.generateNonDocumentNodeDataChildren(refreshedOuterNode);
504+
const outerMembers = VisualizationService.generateNonDocumentNodeDataChildren(outerNode);
516505
expect(outerMembers.length).toEqual(2);
517506
expect(outerMembers[0]).toBeInstanceOf(ChoiceFieldNodeData);
518507
expect(outerMembers[1].title).toEqual('regularField');

packages/ui/src/services/visualization/visualization.service.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,12 @@ export class VisualizationService {
142142
): NodeData {
143143
const selectedMember =
144144
field.selectedMemberIndex === undefined ? undefined : field.fields?.[field.selectedMemberIndex];
145+
146+
if (selectedMember?.wrapperKind && selectedMember.selectedMemberIndex !== undefined) {
147+
const innerSpec = selectedMember.wrapperKind === 'choice' ? CHOICE_WRAPPER : ABSTRACT_WRAPPER;
148+
return VisualizationService.doGenerateNodeDataFromWrapperField(parent, selectedMember, mappings, innerSpec);
149+
}
150+
145151
const nodeField = selectedMember ?? field;
146152
if (parent.isSource) {
147153
const node = spec.createSourceNode(parent, nodeField);

0 commit comments

Comments
 (0)