ljmotta commented on code in PR #2681:
URL:
https://github.com/apache/incubator-kie-tools/pull/2681#discussion_r1876847443
##########
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##########
@@ -1073,6 +1077,8 @@ export function DecisionTableExpression({
shouldRenderRowIndexColumn={true}
shouldShowRowsInlineControls={true}
shouldShowColumnsInlineControls={true}
+ evaluationHitsCountColumnIndex={0}
Review Comment:
I get the `0` here, but it would be good to add a comment too.
##########
packages/boxed-expression-component/src/expressions/ConditionalExpression/ConditionalExpression.tsx:
##########
@@ -247,6 +256,8 @@ export function ConditionalExpression({
shouldRenderRowIndexColumn={false}
shouldShowRowsInlineControls={false}
shouldShowColumnsInlineControls={false}
+ evaluationHitsCountColumnIndex={1}
Review Comment:
Why `1`? Could you please add a comment here? Or make it a variable with a
very good name.
##########
packages/boxed-expression-component/src/api/BeeTable.ts:
##########
@@ -97,6 +97,10 @@ export interface BeeTableProps<R extends object> {
shouldShowColumnsInlineControls: boolean;
resizerStopBehavior: ResizerStopBehavior;
lastColumnMinWidth?: number;
+ /** Index of the column, where the evaluation hits count should be
displayed. If not set, evaluation hits count number is not shown. */
+ evaluationHitsCountColumnIndex?: number;
+ /** Method should return true for table rows, that can display evaluation
hits count, false otherwise. If not set, BeeTableBody defaults to false. */
+ getEvaluationHitsCountSupportedByRow?: (row: ReactTable.Row<R>) => boolean;
Review Comment:
The name is a little strange for me. The idea is to the get if a row
supports (or not) the "EvaluationHitsCount". I don't think the "ByRow" makes
sense, so:
- `isEvaluationHitCountSupported(row: Row)`: The `is` tells the method will
be boolean
- `supportsEvaluationHitCount(row: Row)`
Another idea is to use the `args: { row: Row }`, but I don't think is
necessary, as we just have one parameter.
##########
packages/boxed-expression-component/src/BoxedExpressionEditorContext.tsx:
##########
@@ -34,6 +34,7 @@ export interface BoxedExpressionEditorContextType {
pmmlDocuments?: PmmlDocument[];
dataTypes: DmnDataType[];
isReadOnly?: boolean;
+ evaluationHitsCountPerId?: Map<string, number>;
Review Comment:
```suggestion
evaluationHitsCountById?: Map<string, number>;
```
To keep consistency with other variables in the codebase.
##########
packages/boxed-expression-component/src/table/BeeTable/BeeTableTd.tsx:
##########
@@ -225,6 +231,12 @@ export function BeeTableTd<R extends object>({
return onDataCellKeyUp?.(column.id);
}, [column.id, onDataCellKeyUp]);
+ const evaluationHitsCountBadgeClassName = canDisplayEvaluationHitsCountBadge
+ ? (evaluationHitsCount ?? 0) > 0
+ ? "evaluation-hits-count-badge-colored"
+ : "evaluation-hits-count-badge-non-colored"
+ : "";
Review Comment:
Please, use an `useMemo` hook here.
##########
packages/boxed-expression-component/src/table/BeeTable/BeeTableBody.tsx:
##########
@@ -72,15 +77,29 @@ export function BeeTableBody<R extends object>({
lastColumnMinWidth,
rowWrapper,
isReadOnly,
+ evaluationHitsCountColumnIndex,
+ getEvaluationHitsCountSupportedByRow,
}: BeeTableBodyProps<R>) {
+ const { evaluationHitsCountPerId } = useBoxedExpressionEditor();
+
const renderRow = useCallback(
(row: ReactTable.Row<R>, rowIndex: number) => {
reactTableInstance.prepareRow(row);
+ const rowKey = getRowKey(row);
+ const isEvalationHitsCountSupportedByRow: boolean =
getEvaluationHitsCountSupportedByRow?.(row) ?? false;
+ const rowEvaluationHitsCount = evaluationHitsCountPerId ?
evaluationHitsCountPerId?.get(rowKey) ?? 0 : undefined;
+ const canDisplayEvaluationHitsCountRowOverlay =
+ rowEvaluationHitsCount !== undefined &&
isEvalationHitsCountSupportedByRow === true;
+ const rowClassName = canDisplayEvaluationHitsCountRowOverlay
+ ? rowKey + (rowEvaluationHitsCount > 0 ? "
evaluation-hits-count-row-overlay" : "")
+ : rowKey;
Review Comment:
This part is a little dense, a lot of information here.
- Could you please inline the `isEvalationHitsCountSupportedByRow` as it's
used only one time?
- I'm not sure about this one, but can you force the ternary to be written
in multiple lines?
- Could you please add a line break between? As multiple ternaries can be
very difficult to read.
--
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]
