You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "jomarko (via GitHub)" <gi...@apache.org> on 2024/10/11 11:09:23 UTC
[PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight eva… [incubator-kie-tools]
jomarko opened a new pull request, #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657
…luation results'
Closes: https://github.com/apache/incubator-kie-issues/issues/1527
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1799327573
##########
packages/dmn-editor/src/store/Store.ts:
##########
@@ -110,6 +110,7 @@ export interface State {
enableExecutionHitsHighlights: boolean;
enableDataTypesToolbarOnNodes: boolean;
enableCustomNodeStyles: boolean;
+ evaluationHighlights: boolean;
Review Comment:
there is existing **enableExecutionHitsHighlights**, however not sure if we should:
- keep it
- replace it with **evaluationHighlights**
- other action
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1799093950
##########
packages/dmn-editor/src/overlaysPanel/OverlaysPanel.tsx:
##########
@@ -159,6 +161,27 @@ export function OverlaysPanel({ availableHeight }: OverlaysPanelProps) {
}
/>
</FormGroup>
+ <FormGroup
+ label={"Enable evaluation highlights"}
+ labelIcon={
+ <Tooltip
+ content={"Enable evaluation highlights on nodes(decisions?), decision tables and conditional expressions"}
Review Comment:
I will appreciate any hints, how to update this tooltip text. Not sure how much information it should provide.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1801793826
##########
packages/dmn-editor/src/evaluationHighlights/EvaluationHighlightsBadge.tsx:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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 { useDmnEditorStore, useDmnEditorStoreApi } from "../store/StoreContext";
+
+export function EvaluationHighlightsBadge() {
+ const dmnEditorStoreApi = useDmnEditorStoreApi();
+ const isEvaluationHighlights = useDmnEditorStore((s) => s.diagram.overlays.enableEvaluationHighlights);
Review Comment:
```suggestion
const isEvaluationHighlightsEnabled = useDmnEditorStore((s) => s.diagram.overlays.enableEvaluationHighlights);
```
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#issuecomment-2421702730
@tiagobento good feedback, I will try to incorporate it.
> Border looks a little off, and the tooltip says (beta), but it isn't.
I will remove the "(beta)". Oh, you are correct, I haven't noticed the border off placement. It is an example that author eyes are focused on other things than reviewer eyes. Thank you for spotting.
> Also, I think the idea of having this as a https://www.patternfly.org/components/label/ instead of a "Button" is important to give it a different look compared to the other buttons, since this this is not really a button, but more like an indicator that the Evaluation highlights are turned on.
Ok, probably this I didn't understand from mockup. There it seemed to me as UI element with same semantic as other elements in that area, that is why I used the button. I will incorporate your proposal.
> Same is true for the toggle on BEE.
Sure.
> We also need to be mindful of the case we choose when writing text on-screen. I always prefer to capitalize only the first word, not all words, so the label would read "Evaluation highlights: on/off", instead of "Evaluation Highlights: On/Off".
I do not have preference about this. I don't mind follow your suggestion.
> Another thing that caught my eye was the little displacement that happened when the I toggled between on and off on BEE, because of difference in text size. Being mindful of layout shifts is also very important for a good experience when using a web app.
WIll try to fix the size of the new toggle/label. However, that will probably won't work with potential i18n in future.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#issuecomment-2417467714
Other than that ^ looks good to go! :)
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1799776234
##########
packages/dmn-editor/src/overlaysPanel/OverlaysPanel.tsx:
##########
@@ -159,6 +161,27 @@ export function OverlaysPanel({ availableHeight }: OverlaysPanelProps) {
}
/>
</FormGroup>
+ <FormGroup
+ label={"Enable evaluation highlights"}
+ labelIcon={
+ <Tooltip
+ content={"Enable evaluation highlights on nodes(decisions?), decision tables and conditional expressions"}
Review Comment:
"Enable highlighting Decision Table rules and Boxed Conditional Expression branches based on evaluation results, also showing success/error status badges on Decision nodes."
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#issuecomment-2529119381
@jomarko The code looks great, but I found an issue in the UI. Using the toggle makes the overflow Y appear. This only happens when the Overlay panel is close to the bottom of the Editor, which is the case for me in the default zoom (100%).
https://github.com/user-attachments/assets/2c0a0c2d-5106-48c6-b19b-11216cc57eac
BUG (100%):
OK (90%):
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1799776234
##########
packages/dmn-editor/src/overlaysPanel/OverlaysPanel.tsx:
##########
@@ -159,6 +161,27 @@ export function OverlaysPanel({ availableHeight }: OverlaysPanelProps) {
}
/>
</FormGroup>
+ <FormGroup
+ label={"Enable evaluation highlights"}
+ labelIcon={
+ <Tooltip
+ content={"Enable evaluation highlights on nodes(decisions?), decision tables and conditional expressions"}
Review Comment:
"Enable highlighting Decision Table rules and Boxed Conditional Expression branches based on evaluation results, also showing a success/error status badge on Decision nodes."
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#issuecomment-2421187915
<img width="417" alt="image" src="https://github.com/user-attachments/assets/c4d4247f-8ade-46db-ad35-18a49624425b">
Border looks a little off, and the tooltip says (beta), but it isn't.
Also, I think the idea of having this as a https://www.patternfly.org/components/label/ instead of a "Button" is important to give it a different look compared to the other buttons, since this this is not really a button, but more like an indicator that the Evaluation highlights are turned on.
Same is true for the toggle on BEE.
We also need to be mindful of the case we choose when writing text on-screen. I always prefer to capitalize only the first word, not all words, so the label would read "Evaluation highlights: on/off", instead of "Evaluation Highlights: On/Off".
Another thing that caught my eye was the little displacement that happened when the I toggled between on and off on BEE, because of difference in text size. Being mindful of layout shifts is also very important for a good experience when using a web app.
Please consider my feedback, and let me know if you have questions!
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#issuecomment-2422087766
Changes pushed:
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko merged PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1799748396
##########
packages/dmn-editor/src/store/Store.ts:
##########
@@ -110,6 +110,7 @@ export interface State {
enableExecutionHitsHighlights: boolean;
enableDataTypesToolbarOnNodes: boolean;
enableCustomNodeStyles: boolean;
+ evaluationHighlights: boolean;
Review Comment:
`enableExecutionHitsHighlights` was supposed to be exactly what `evaluationHighlights` is going to be. Please remove `enableExecutionHitsHighlights` and continue with `evaluationHighlights`. Note that `enableEvaluationHighlights` is a better name.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1800715141
##########
packages/dmn-editor/src/overlaysPanel/OverlaysPanel.tsx:
##########
@@ -159,6 +161,27 @@ export function OverlaysPanel({ availableHeight }: OverlaysPanelProps) {
}
/>
</FormGroup>
+ <FormGroup
+ label={"Enable evaluation highlights"}
+ labelIcon={
+ <Tooltip
+ content={"Enable evaluation highlights on nodes(decisions?), decision tables and conditional expressions"}
Review Comment:
thank you
##########
packages/dmn-editor/src/store/Store.ts:
##########
@@ -110,6 +110,7 @@ export interface State {
enableExecutionHitsHighlights: boolean;
enableDataTypesToolbarOnNodes: boolean;
enableCustomNodeStyles: boolean;
+ evaluationHighlights: boolean;
Review Comment:
thank you, applied
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#1527: DMN Editor: Create new overlay option 'Highlight evaluation results' [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2657:
URL: https://github.com/apache/incubator-kie-tools/pull/2657#discussion_r1804209506
##########
packages/dmn-editor/src/evaluationHighlights/EvaluationHighlightsBadge.tsx:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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 { useDmnEditorStore, useDmnEditorStoreApi } from "../store/StoreContext";
+
+export function EvaluationHighlightsBadge() {
+ const dmnEditorStoreApi = useDmnEditorStoreApi();
+ const isEvaluationHighlights = useDmnEditorStore((s) => s.diagram.overlays.enableEvaluationHighlights);
Review Comment:
thank you @tiagobento, I agree, it is a better name, changes pushed
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org