Copilot commented on code in PR #3716: URL: https://github.com/apache/incubator-kie-tools/pull/3716#discussion_r3802983606
########## packages/dmn-editor/tests-e2e/__fixtures__/propertiesPanel/edgePropertiesPanel.ts: ########## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Page } from "@playwright/test"; +import { Diagram } from "../diagram"; +import { EdgeType } from "../edges"; +import { PropertiesPanelBase } from "./propertiesPanelBase"; + +export const EDGE_TITLE: Record<EdgeType, string> = { + [EdgeType.INFORMATION_REQUIREMENT]: "Information Requirement", + [EdgeType.KNOWLEDGE_REQUIREMENT]: "Knowledge Requirement", + [EdgeType.AUTHORITY_REQUIREMENT]: "Authority Requirement", + [EdgeType.ASSOCIATION]: "Association", +}; + +export class EdgePropertiesPanel extends PropertiesPanelBase { + constructor( + public diagram: Diagram, + public page: Page + ) { + super(diagram, page); + } + + /** Returns the text content of the panel header title (e.g. "Information Requirement"). */ + public async getTitle() { + return await this.panel().locator(".kie-dmn-editor--properties-panel-header-title").first().textContent(); + } + + /** Returns the current value of the Description textarea. */ + public async getDescription() { + return await this.panel().getByLabel("Description").inputValue(); + } + + /** Sets the Description textarea to the given value. */ + public async setDescription(args: { newDescription: string }) { + await this.panel().getByLabel("Description").fill(args.newDescription); + await this.panel().getByLabel("Description").press("Tab"); + } + + /** Returns the read-only edge ID shown in the ClipboardCopy field. */ + public async getId() { + return await this.panel().getByLabel("Copyable input").inputValue(); Review Comment: Using `getByLabel(\"Copyable input\")` is brittle because it depends on PatternFly's internal default accessible label and/or locale. Once the UI sets a stable accessible name/`id` for the edge ID field, update this to target that stable label/test id instead to reduce flaky failures across library upgrades or localization changes. ########## packages/dmn-editor/src/propertiesPanel/SingleEdgeProperties.tsx: ########## @@ -0,0 +1,266 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as React from "react"; +import { useMemo, useState } from "react"; +import { Button, ButtonVariant } from "@patternfly/react-core/dist/js/components/Button"; +import { Form, FormSection, FormGroup } from "@patternfly/react-core/dist/js/components/Form"; +import { ClipboardCopy } from "@patternfly/react-core/dist/js/components/ClipboardCopy"; +import { TextArea } from "@patternfly/react-core/dist/js/components/TextArea"; +import { TimesIcon } from "@patternfly/react-icons/dist/js/icons/times-icon"; +import { + DMN_LATEST__tAssociation, + DMN_LATEST__tAuthorityRequirement, + DMN_LATEST__tInformationRequirement, + DMN_LATEST__tKnowledgeRequirement, +} from "@kie-tools/dmn-marshaller"; +import { Normalized } from "@kie-tools/dmn-marshaller/dist/normalization/normalize"; +import { useDmnEditorStore, useDmnEditorStoreApi } from "../store/StoreContext"; +import { useExternalModels } from "../includedModels/DmnEditorDependenciesContext"; +import { useSettings } from "../settings/DmnEditorSettingsContext"; +import { useDmnEditorI18n } from "../i18n"; +import { PropertiesPanelHeader } from "./PropertiesPanelHeader"; +import { EDGE_TYPES } from "../diagram/edges/EdgeTypes"; +import { + InformationRequirementPath, + KnowledgeRequirementPath, + AuthorityRequirementPath, + AssociationPath, +} from "../diagram/edges/Edges"; + +const handleButtonSize = 34; +const svgViewboxPadding = Math.sqrt(Math.pow(handleButtonSize, 2) / 2) - handleButtonSize / 2; +const edgeSvgViewboxSize = 25; + +export function SingleEdgeProperties({ edgeId }: { edgeId: string }) { + const { i18n } = useDmnEditorI18n(); + const dmnEditorStoreApi = useDmnEditorStoreApi(); + const { externalModelsByNamespace } = useExternalModels(); + const settings = useSettings(); + + const edge = useDmnEditorStore((s) => + s.computed(s).getDiagramData(externalModelsByNamespace).selectedEdgesById.get(edgeId) + ); + + const description = useDmnEditorStore((s) => { + if (!edge?.data) { + return undefined; + } + const { requirementType, index, id: parentId } = edge.data.dmnObject; + const defs = s.dmn.model.definitions; + + if (requirementType === "association") { + return (defs.artifact![index] as Normalized<DMN_LATEST__tAssociation>).description?.__$$text ?? ""; + } + + const parentIndex = (defs.drgElement ?? []).findIndex((e) => e["@_id"] === parentId); + if (parentIndex < 0) { + return ""; + } + const parent = defs.drgElement![parentIndex]; + + if (requirementType === "informationRequirement") { + return ( + (parent as { informationRequirement?: Normalized<DMN_LATEST__tInformationRequirement>[] }) + .informationRequirement?.[index]?.description?.__$$text ?? "" + ); + } + if (requirementType === "knowledgeRequirement") { + return ( + (parent as { knowledgeRequirement?: Normalized<DMN_LATEST__tKnowledgeRequirement>[] }).knowledgeRequirement?.[ + index + ]?.description?.__$$text ?? "" + ); + } + if (requirementType === "authorityRequirement") { + return ( + (parent as { authorityRequirement?: Normalized<DMN_LATEST__tAuthorityRequirement>[] }).authorityRequirement?.[ + index + ]?.description?.__$$text ?? "" + ); + } + return ""; + }); + + const [isSectionExpanded, setSectionExpanded] = useState<boolean>(true); + + const Icon = useMemo(() => { + if (!edge) { + return () => null; + } + switch (edge.type) { + case EDGE_TYPES.informationRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <InformationRequirementPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},0`} /> + </svg> + ); + case EDGE_TYPES.knowledgeRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <KnowledgeRequirementPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},0`} /> + </svg> + ); + case EDGE_TYPES.authorityRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <AuthorityRequirementPath + d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},2`} + centerToConnectionPoint={false} + /> + </svg> + ); + case EDGE_TYPES.association: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <AssociationPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize},0`} strokeWidth={2} /> + </svg> + ); + default: + return () => null; + } + }, [edge]); + + const title = useMemo(() => { + if (!edge) { + return i18n.propertiesPanel.edge; + } + switch (edge.type) { + case EDGE_TYPES.informationRequirement: + return i18n.propertiesPanel.informationRequirement; + case EDGE_TYPES.knowledgeRequirement: + return i18n.propertiesPanel.knowledgeRequirement; + case EDGE_TYPES.authorityRequirement: + return i18n.propertiesPanel.authorityRequirement; + case EDGE_TYPES.association: + return i18n.propertiesPanel.association; + default: + return i18n.propertiesPanel.edge; + } + }, [edge, i18n]); + + if (!edge) { + return <>{i18n.propertiesPanel.edgeNotFound(edgeId)}</>; + } + + return ( + <Form> + <FormSection + className={!isSectionExpanded ? "kie-dmn-editor--single-node-properties-title-colapsed" : ""} + title={ + <PropertiesPanelHeader + expands={true} + fixed={true} + isSectionExpanded={isSectionExpanded} + toogleSectionExpanded={() => setSectionExpanded((prev) => !prev)} Review Comment: There are likely typos in identifiers: `...title-colapsed` (commonly `collapsed`) and `toogleSectionExpanded` (commonly `toggleSectionExpanded`). If the CSS class / prop name is not intentionally misspelled to match existing definitions, this will prevent the collapsed styling from applying and/or risk an API mismatch with `PropertiesPanelHeader`. Consider aligning spelling with the existing CSS/prop naming (or renaming consistently across the codebase if this is a new API). ########## packages/dmn-editor/tests-e2e/drgRequirements/edgeProperties.spec.ts: ########## @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { test, expect } from "../__fixtures__/base"; +import { DefaultNodeName, NodeType } from "../__fixtures__/nodes"; +import { EdgeType } from "../__fixtures__/edges"; +import { EDGE_TITLE } from "../__fixtures__/propertiesPanel/edgePropertiesPanel"; + +test.beforeEach(async ({ editor }) => { + await editor.open(); +}); + +test.describe("Edge Properties Panel", () => { + test.describe("Information Requirement edge", () => { + test.beforeEach(async ({ palette, nodes }) => { + await palette.dragNewNode({ type: NodeType.INPUT_DATA, targetPosition: { x: 100, y: 100 } }); + await palette.dragNewNode({ type: NodeType.DECISION, targetPosition: { x: 100, y: 300 } }); + await nodes.dragNewConnectedEdge({ + type: EdgeType.INFORMATION_REQUIREMENT, + from: DefaultNodeName.INPUT_DATA, + to: DefaultNodeName.DECISION, + }); + }); + + test("should show 'Information Requirement' as the panel title when the edge is selected", async ({ + edges, + edgePropertiesPanel, + }) => { + await edges.select({ from: DefaultNodeName.INPUT_DATA, to: DefaultNodeName.DECISION }); + await edgePropertiesPanel.open(); + + const title = await edgePropertiesPanel.getTitle(); + expect(title?.trim()).toBe(EDGE_TITLE[EdgeType.INFORMATION_REQUIREMENT]); + }); + + test("should show the edge ID in the properties panel", async ({ edges, edgePropertiesPanel }) => { + await edges.select({ from: DefaultNodeName.INPUT_DATA, to: DefaultNodeName.DECISION }); + await edgePropertiesPanel.open(); + + const id = await edgePropertiesPanel.getId(); + expect(id).toBeTruthy(); + expect(id).toMatch(/^_/); Review Comment: Asserting the edge ID starts with `_` is very specific to the current ID generator and may break if ID generation changes while still being valid. To make the test more resilient, assert a more general DMN/XML ID shape (or simply non-empty + stable across readback), rather than enforcing a leading underscore. ########## packages/dmn-editor/src/propertiesPanel/SingleEdgeProperties.tsx: ########## @@ -0,0 +1,266 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as React from "react"; +import { useMemo, useState } from "react"; +import { Button, ButtonVariant } from "@patternfly/react-core/dist/js/components/Button"; +import { Form, FormSection, FormGroup } from "@patternfly/react-core/dist/js/components/Form"; +import { ClipboardCopy } from "@patternfly/react-core/dist/js/components/ClipboardCopy"; +import { TextArea } from "@patternfly/react-core/dist/js/components/TextArea"; +import { TimesIcon } from "@patternfly/react-icons/dist/js/icons/times-icon"; +import { + DMN_LATEST__tAssociation, + DMN_LATEST__tAuthorityRequirement, + DMN_LATEST__tInformationRequirement, + DMN_LATEST__tKnowledgeRequirement, +} from "@kie-tools/dmn-marshaller"; +import { Normalized } from "@kie-tools/dmn-marshaller/dist/normalization/normalize"; +import { useDmnEditorStore, useDmnEditorStoreApi } from "../store/StoreContext"; +import { useExternalModels } from "../includedModels/DmnEditorDependenciesContext"; +import { useSettings } from "../settings/DmnEditorSettingsContext"; +import { useDmnEditorI18n } from "../i18n"; +import { PropertiesPanelHeader } from "./PropertiesPanelHeader"; +import { EDGE_TYPES } from "../diagram/edges/EdgeTypes"; +import { + InformationRequirementPath, + KnowledgeRequirementPath, + AuthorityRequirementPath, + AssociationPath, +} from "../diagram/edges/Edges"; + +const handleButtonSize = 34; +const svgViewboxPadding = Math.sqrt(Math.pow(handleButtonSize, 2) / 2) - handleButtonSize / 2; +const edgeSvgViewboxSize = 25; + +export function SingleEdgeProperties({ edgeId }: { edgeId: string }) { + const { i18n } = useDmnEditorI18n(); + const dmnEditorStoreApi = useDmnEditorStoreApi(); + const { externalModelsByNamespace } = useExternalModels(); + const settings = useSettings(); + + const edge = useDmnEditorStore((s) => + s.computed(s).getDiagramData(externalModelsByNamespace).selectedEdgesById.get(edgeId) + ); + + const description = useDmnEditorStore((s) => { + if (!edge?.data) { + return undefined; + } + const { requirementType, index, id: parentId } = edge.data.dmnObject; + const defs = s.dmn.model.definitions; + + if (requirementType === "association") { + return (defs.artifact![index] as Normalized<DMN_LATEST__tAssociation>).description?.__$$text ?? ""; + } + + const parentIndex = (defs.drgElement ?? []).findIndex((e) => e["@_id"] === parentId); + if (parentIndex < 0) { + return ""; + } + const parent = defs.drgElement![parentIndex]; + + if (requirementType === "informationRequirement") { + return ( + (parent as { informationRequirement?: Normalized<DMN_LATEST__tInformationRequirement>[] }) + .informationRequirement?.[index]?.description?.__$$text ?? "" + ); + } + if (requirementType === "knowledgeRequirement") { + return ( + (parent as { knowledgeRequirement?: Normalized<DMN_LATEST__tKnowledgeRequirement>[] }).knowledgeRequirement?.[ + index + ]?.description?.__$$text ?? "" + ); + } + if (requirementType === "authorityRequirement") { + return ( + (parent as { authorityRequirement?: Normalized<DMN_LATEST__tAuthorityRequirement>[] }).authorityRequirement?.[ + index + ]?.description?.__$$text ?? "" + ); + } + return ""; + }); + + const [isSectionExpanded, setSectionExpanded] = useState<boolean>(true); + + const Icon = useMemo(() => { + if (!edge) { + return () => null; + } + switch (edge.type) { + case EDGE_TYPES.informationRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <InformationRequirementPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},0`} /> + </svg> + ); + case EDGE_TYPES.knowledgeRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <KnowledgeRequirementPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},0`} /> + </svg> + ); + case EDGE_TYPES.authorityRequirement: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <AuthorityRequirementPath + d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize - 2},2`} + centerToConnectionPoint={false} + /> + </svg> + ); + case EDGE_TYPES.association: + return () => ( + <svg + viewBox={`0 0 ${edgeSvgViewboxSize} ${edgeSvgViewboxSize}`} + className={"kie-dmn-editor--round-svg-container"} + style={{ padding: `${svgViewboxPadding}px` }} + > + <AssociationPath d={`M2,${edgeSvgViewboxSize - 2} L${edgeSvgViewboxSize},0`} strokeWidth={2} /> + </svg> + ); + default: + return () => null; + } + }, [edge]); + + const title = useMemo(() => { + if (!edge) { + return i18n.propertiesPanel.edge; + } + switch (edge.type) { + case EDGE_TYPES.informationRequirement: + return i18n.propertiesPanel.informationRequirement; + case EDGE_TYPES.knowledgeRequirement: + return i18n.propertiesPanel.knowledgeRequirement; + case EDGE_TYPES.authorityRequirement: + return i18n.propertiesPanel.authorityRequirement; + case EDGE_TYPES.association: + return i18n.propertiesPanel.association; + default: + return i18n.propertiesPanel.edge; + } + }, [edge, i18n]); + + if (!edge) { + return <>{i18n.propertiesPanel.edgeNotFound(edgeId)}</>; + } + + return ( + <Form> + <FormSection + className={!isSectionExpanded ? "kie-dmn-editor--single-node-properties-title-colapsed" : ""} + title={ + <PropertiesPanelHeader + expands={true} + fixed={true} + isSectionExpanded={isSectionExpanded} + toogleSectionExpanded={() => setSectionExpanded((prev) => !prev)} + icon={<Icon />} + title={title} + action={ + <Button + title={i18n.close} + variant={ButtonVariant.plain} + onClick={() => { + dmnEditorStoreApi.setState((state) => { + state.boxedExpressionEditor.propertiesPanel.isOpen = false; + state.diagram.propertiesPanel.isOpen = false; + }); + }} + > + <TimesIcon /> + </Button> + } + /> + } + > + {isSectionExpanded && ( + <FormSection style={{ paddingLeft: "20px" }}> + <FormGroup label={i18n.propertiesPanel.description}> + <TextArea + aria-label={"Description"} + isDisabled={settings.isReadOnly} + value={description ?? ""} + onChange={(_event, newDescription) => { + dmnEditorStoreApi.setState((state) => { + const { requirementType, index, id: parentId } = edge.data!.dmnObject; + const defs = state.dmn.model.definitions; + + if (requirementType === "association") { + (defs.artifact![index] as Normalized<DMN_LATEST__tAssociation>).description = { + __$$text: newDescription, + }; + return; + } + + const parentIndex = (defs.drgElement ?? []).findIndex((e) => e["@_id"] === parentId); + if (parentIndex < 0) { + return; + } + const parent = defs.drgElement![parentIndex]; + + if (requirementType === "informationRequirement") { + ( + parent as { informationRequirement?: Normalized<DMN_LATEST__tInformationRequirement>[] } + ).informationRequirement![index].description = { __$$text: newDescription }; + } else if (requirementType === "knowledgeRequirement") { + ( + parent as { knowledgeRequirement?: Normalized<DMN_LATEST__tKnowledgeRequirement>[] } + ).knowledgeRequirement![index].description = { __$$text: newDescription }; + } else if (requirementType === "authorityRequirement") { + ( + parent as { authorityRequirement?: Normalized<DMN_LATEST__tAuthorityRequirement>[] } + ).authorityRequirement![index].description = { __$$text: newDescription }; + } + }); + }} + placeholder={i18n.propertiesPanel.descriptionPlaceholder} + style={{ resize: "vertical", minHeight: "40px" }} + rows={6} + /> + </FormGroup> + <FormGroup label={i18n.propertiesPanel.id}> + <ClipboardCopy + isReadOnly={true} + hoverTip={i18n.propertiesPanel.copy} + clickTip={i18n.propertiesPanel.copied} + > + {edgeId} + </ClipboardCopy> Review Comment: `ClipboardCopy` currently doesn't set an explicit accessible name/label, which makes it hard to target reliably in E2E tests and can be suboptimal for assistive tech. Consider adding an explicit `aria-label` (e.g. based on `i18n.propertiesPanel.id`) or associating it via `FormGroup fieldId` to avoid depending on PatternFly's internal default label. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
