kie-issues#1741: Sandbox: Add “Open Boxed Expression Editor” button in the DMN Runner table output columns #2849

jomarko · 2025-01-10T13:31:46Z

The dmn runner outputs table introduces a button in the header cell top right corner for navigating into the bound expression. It means the boxed expression editor is open for the given dmn runner result.

…n in the DMN Runner form output cards Closes: apache/incubator-kie-issues#1595

jomarko · 2025-01-28T10:44:00Z

fixing column resize feature

jomarko · 2025-01-29T09:16:41Z

@tiagobento @kbowers-ibm @ljmotta or anyone else, I will write down my analysis of the problem #2849 (comment), it is the thing that runner output columns are not resizable, there is missing the resize handler. It was introduced by changes regarding readOnly mode https://github.com/apache/incubator-kie-tools/pull/2538/files#diff-ab6fc87d76da643cd7a7f3cd903f077619837939303defc50b2d0e208a2d5ca5.

The missing resize handler in dmn runner outputs columns is a problem because of this two things

Too long header text

Makes header unreadable and in combination with new arrow up button quite 'messy'.

Too long data text

Makes data unreadable

So here are some options I see. Please comment them or bring other so we can decide what option is the best.

01 'Weak' ReadOnly mode

We could change the isReadOnly of StandaloneBeeTable to be slightly 'weak'. I mean, we could allow to display resize handler even if isReadOnly is set to true

Pros

it would fix the issue of unreadable header or data if the text is too long

Cons

it decreases the level 'readonly' feeling. for example, readonly in google spreadsheets does not allow to resize column, if I am not wrong
it affects more places, not just dmn runner outputs. for example opened included decision node

02 'allowResizing' property

We could introduce additional property allowResizing next to isReadOnly in StandaloneBeeTable

Pros

we would be able to fix just the dmn runner outputs scenario without an effect on other StandaloneBeeTable usages

Cons

it increases StandaloneBeeTable props complexity

03 'width' computation

We could reuse the column width computation for given text as we have here: https://github.com/apache/incubator-kie-tools/blob/main/packages/dmn-editor/src/boxedExpressions/getDefaultColumnWidth.tsx directly in DmnRunnerOutputsTable when we define beeTableColumns variable.

Pros

We do not need to touch StandaloneBeeTable code
We keep strong readonly 'feeling' on all StandaloneBeeTable usages

Cons

we fix only the scenario of a long text in the header, if a long text is in data cell, it still remains unreadable

kbowers-ibm · 2025-01-29T09:38:11Z

Thanks for the detailed analysis @jomarko - you've clearly put a lot of thought and effort into this.
My vote would be for Option 3 as I think it would probably be part of any refactoring efforts on the DMN Runner table anyway. This then allows you to continue with your task, and while the con is an issue, it's not like it's introduced by/related to your PR. I think best would be to fix the header as you describe and raise the issue of unreadable long text in data cell as tech debt if it's not already an existing issue.

ljmotta

@jomarko Thanks for the PR, I've made some comments inline. Please let me know if you have any questions or need further explanation.

ljmotta · 2025-01-29T14:20:14Z

packages/online-editor/src/dmnRunner/DmnRunnerTable.tsx

+                            !isLegacyDmnEditor
+                              ? (nodeId: string) => {
+                                  const newDmnEditorEnvelopeApi = props.editor?.getEnvelopeServer()
+                                    .envelopeApi as unknown as MessageBusClientApi<NewDmnEditorEnvelopeApi>;


Why the as unknown is required?

ljmotta · 2025-01-29T14:23:56Z

packages/online-editor/src/dmnRunner/DmnRunnerTable.tsx


-export function DmnRunnerTable() {
+export function DmnRunnerTable(props: { editor: EmbeddedEditorRef | undefined }) {


I'm don't think we want to have an undefined editor as props of the DmnRunnerTable. Also, the getEnvelopeServer() is the only property of editor that is used, which could be accordlying typed to avoid casting.

ljmotta · 2025-01-29T14:24:22Z

packages/online-editor/src/editor/EditorPage.tsx

@@ -468,6 +468,7 @@ Error details: ${err}`);
                workspaceFile={file.workspaceFile}
                workspaces={workspaces}
                dmnLanguageService={dmnLanguageService}
+                editor={editor}


As mentioned, we could pass the editorEnvelopeServer.

ljmotta · 2025-01-29T14:25:11Z

packages/online-editor/src/editor/EditorPageDockContextProvider.tsx

@@ -186,7 +189,7 @@ export function EditorPageDockContextProvider({
      case PanelId.DMN_RUNNER_TABLE:
        return (
          <DmnRunnerErrorBoundary>
-            <DmnRunnerTable />
+            <DmnRunnerTable editor={editor} />


ljmotta · 2025-01-29T14:27:38Z

packages/boxed-expression-component/src/BoxedExpressionEditorContext.css

@@ -99,3 +99,9 @@
  font-size: smaller;
  color: #1b515f;
 }
+
+.kie-tools--bee--header-cell-element-extension {


I don't want to be picky here, but for this particular case, we don't use the kie-tools--bee prefix. For the boxed-expression-component we are using multiple classes.

I am sorry I used wrong prefix, as you mentioned and I noticed, we use multiple prefixes, and I am sorry, I didn't figure out from the codebase which prefix should I use in my PR.

Is header-cell-element-extension fine for you? I see we have header-cell or header-cell-info already in use.

ljmotta · 2025-01-29T14:43:35Z

packages/unitables-dmn/src/DmnRunnerOutputsTable.tsx

+  const onOpenBoxedExpressionHeaderButtonClick = useCallback(
+    (clickedDecisionId: string) => {
+      (results?.[0] ?? []).flatMap(({ decisionId: resultDecisionId }) => {
+        if (clickedDecisionId === resultDecisionId) {
+          openBoxedExpressionEditor?.(resultDecisionId);
+        }
+      });
+    },
+    [openBoxedExpressionEditor, results]
+  );


Well, everytime we click on the button we will search the array to check if decisionId is part of the results. Why not save the result decisionIds in a set?

const resultsDecisionIds = useMemo(() => results?.[0]?.reduce((set, result) => { set.add(result.decisionId); return set; }, new Set()), [results]); ... onClick={() => resultsDecisionIds.has(decisionId) && openBoxedExpressionEditor?.(decisionId)}

(I didn't test the code above)

cool, I like this incorporated

ljmotta · 2025-01-29T14:58:35Z

@jomarko I agree with @kbowers-ibm . Both option 1 and 2 would require to persist the column size, or closing and opening the DMN Runner Table would reset to the original size.

jomarko · 2025-01-30T13:09:22Z

tiagobento · 2025-01-31T21:58:00Z

packages/unitables-dmn/src/DmnRunnerOutputsTable.tsx

+   * Wrapping 'getDefaultColumnWidth' to add additional space for the '<ArrowUp />' icon in the runner outputs columns.
+   */
+  const getDefaultDmnRunnerOutputColumnWidth = useCallback((label, dataType) => {
+    return getDefaultColumnWidth({ name: label, typeRef: dataType }) + 100;


Please extract 100 to a nice constant and give it a long and descriptive name 😄

@tiagobento good feedback, thank you. done

kbowers-ibm

This looks great - thanks for the changes to the widths Jozef! This is a much better solution than adding back resizing, as the bug with column sizes not persisting means it would still be messy. Whereas this is simple, effective and looks very clean 🙇

tiagobento · 2025-02-05T22:01:48Z

packages/unitables-dmn/src/DmnRunnerOutputsTable.tsx

+          minWidth: Math.max(
+            (parentColumnMinWidth ?? columnMinWidth) / Object.entries(myObject).length,
+            columnMinWidth
+          ),


Missing hooks dependencies.

@tiagobento thank you for spotting, changes pushed

jomarko added 19 commits November 1, 2024 14:57

kie-issues#1595: Sandbox: Add "Open in Boxed Expression Editor" butto…

f94cd78

…n in the DMN Runner form output cards Closes: apache/incubator-kie-issues#1595

Merge branch 'main' into kie-issues#1595

26163ca

Merge branch 'main' into kie-issues#1595

7a2a0b8

Merge branch 'main' into kie-issues#1595

f5b569e

ChannelApi changes

5c8c363

Merge branch 'main' into kie-issues#1595

b3f30df

unsupported operation

fffdcc5

Remove console.error logs

065a5df

remove async and Promise

37a33f3

Use button

a52fa97

Introduce NewDmn* envelope and channel APIs

e838157

Merge branch 'main' into kie-issues#1595

1235854

Merge branch 'main' into kie-issues#1595

77be060

Do not show button for legacy editor

3cba7d8

documentation

000f526

Add arrow button

148c40b

handlers

de467be

Merge branch 'main' into kie-issues#1741

0291e41

pass proper editor instance

7d93fa1

jomarko added pr: DO NOT MERGE Draft PR, not ready for merging pr: wip PR is still under development area:dmn labels Jan 10, 2025

jomarko added 8 commits January 10, 2025 14:50

output columns

64a5a35

all output columns

1a230c8

Merge branch 'main' into kie-issues#1741

fbfb876

Merge branch 'main' into kie-issues#1741

79487ab

Merge branch 'main' into kie-issues#1595

18c7a92

Fix useMemo dependencies

fdad8ba

Merge branch 'kie-issues#1595' into kie-issues#1741

3b9e17a

useContext

ae3d87c

jomarko added 6 commits January 24, 2025 07:20

Merge branch 'main' into kie-issues#1741

1f488c0

refactor props

ec2e294

do not use decisionId in columnKey

2cc14a0

better callback parameters names

fab6f0a

Merge branch 'main' into kie-issues#1741

13f30d6

use position absolute top right

ed1d20b

kbowers-ibm self-requested a review January 28, 2025 10:16

jomarko marked this pull request as draft January 28, 2025 10:43

Merge branch 'main' into kie-issues#1741

890fa9c

ljmotta requested changes Jan 29, 2025

View reviewed changes

jomarko added 5 commits January 30, 2025 09:22

Merge branch 'main' into kie-issues#1741

2d5e162

refactor arrow up click handler

7bc1cdd

remove kie-tools-bee-- css prefix

4aa8e03

useContext instead of props

81050d2

remove: as unknown

3d9c22a

jomarko added 2 commits January 30, 2025 15:39

getDefaultColumnWidthRefactoring

9e76383

calculate needed width

d3dc87d

jomarko marked this pull request as ready for review January 31, 2025 13:47

tiagobento reviewed Jan 31, 2025

View reviewed changes

jomarko added 2 commits February 3, 2025 08:51

Merge branch 'main' into kie-issues#1741

7373e22

refactor numeric constant to variable constant

4d2fed8

kbowers-ibm approved these changes Feb 4, 2025

View reviewed changes

tiagobento reviewed Feb 5, 2025

View reviewed changes

jomarko added 2 commits February 6, 2025 21:37

Merge branch 'main' into kie-issues#1741

fa7914a

missing hook dependencies

4bb397a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kie-issues#1741: Sandbox: Add “Open Boxed Expression Editor” button in the DMN Runner table output columns #2849

kie-issues#1741: Sandbox: Add “Open Boxed Expression Editor” button in the DMN Runner table output columns #2849

jomarko commented Jan 10, 2025 •

edited

Loading

jomarko commented Jan 28, 2025

jomarko commented Jan 29, 2025

kbowers-ibm commented Jan 29, 2025

ljmotta left a comment

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta Jan 29, 2025

jomarko Jan 30, 2025

ljmotta commented Jan 29, 2025

jomarko commented Jan 30, 2025 •

edited

Loading

tiagobento Jan 31, 2025

jomarko Feb 3, 2025

kbowers-ibm left a comment

tiagobento Feb 5, 2025

jomarko Feb 6, 2025


		export function DmnRunnerTable() {
		export function DmnRunnerTable(props: { editor: EmbeddedEditorRef \| undefined }) {

kie-issues#1741: Sandbox: Add “Open Boxed Expression Editor” button in the DMN Runner table output columns #2849

Are you sure you want to change the base?

kie-issues#1741: Sandbox: Add “Open Boxed Expression Editor” button in the DMN Runner table output columns #2849

Conversation

jomarko commented Jan 10, 2025 • edited Loading

jomarko commented Jan 28, 2025

jomarko commented Jan 29, 2025

Too long header text

Too long data text

01 'Weak' ReadOnly mode

02 'allowResizing' property

03 'width' computation

kbowers-ibm commented Jan 29, 2025

ljmotta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljmotta commented Jan 29, 2025

jomarko commented Jan 30, 2025 • edited Loading

Column width checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbowers-ibm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jomarko commented Jan 10, 2025 •

edited

Loading

jomarko commented Jan 30, 2025 •

edited

Loading