Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DROOLS-4735: [DMN Designer] Grid performance is dire #3004

Merged
merged 5 commits into from
Nov 22, 2019
Merged

DROOLS-4735: [DMN Designer] Grid performance is dire #3004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc984d7
DROOLS-4735: [DMN Designer] Grid performance is dire
Nov 7, 2019
0aada9f
Fix tests. Write new tests.
Nov 11, 2019
b955285
Fix Sonar maintainability issues.
Nov 12, 2019
613e1f3
Fix Sonar test coverage issues.
Nov 13, 2019
6f0d675
Updates following peer review.
Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions ...ava/org/kie/workbench/common/dmn/client/editors/expressions/ExpressionEditorViewImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.kie.workbench.common.dmn.client.editors.expressions.types.ExpressionEditorDefinitions;
import org.kie.workbench.common.dmn.client.resources.i18n.DMNEditorConstants;
import org.kie.workbench.common.dmn.client.session.DMNSession;
import org.kie.workbench.common.dmn.client.widgets.grid.BaseExpressionGrid;
import org.kie.workbench.common.dmn.client.widgets.grid.BoundaryTransformMediator;
import org.kie.workbench.common.dmn.client.widgets.grid.ExpressionGridCache;
import org.kie.workbench.common.dmn.client.widgets.grid.controls.container.CellEditorControlsView;
Expand Down Expand Up @@ -132,10 +131,6 @@ public ExpressionEditorViewImpl(final Anchor returnToLink,
this.domainObjectSelectionEvent = domainObjectSelectionEvent;
}

Optional<BaseExpressionGrid> getExistingEditor() {
return expressionContainerGrid.getExistingEditor();
}

@Override
public void init(final ExpressionEditorView.Presenter presenter) {
this.presenter = presenter;
Expand All @@ -157,7 +152,6 @@ protected void setupGridPanel() {
final Transform transform = new Transform().scale(VP_SCALE);
gridPanel.getElement().setId("dmn_container_" + com.google.gwt.dom.client.Document.get().createUniqueId());
gridPanel.getViewport().setTransform(transform);
gridPanel.add(gridLayer);

final BaseGridWidgetKeyboardHandler handler = new BaseGridWidgetKeyboardHandler(gridLayer);
addKeyboardOperation(handler, new KeyboardOperationEditCell(gridLayer));
Expand Down
13 changes: 5 additions & 8 deletions ...g/kie/workbench/common/dmn/client/editors/expressions/types/dtable/DecisionTableGrid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ protected void doInitialisation() {
public DecisionTableUIModelMapper makeUiModelMapper() {
return new DecisionTableUIModelMapper(this::getModel,
getExpression(),
listSelector);
listSelector,
getExpressionTextLineHeight(getRenderer().getTheme()));
}

@Override
Expand Down Expand Up @@ -248,10 +249,6 @@ private OutputClauseColumn makeOutputClauseColumn(final int index,
return column;
}

private GridRow makeDecisionTableRow() {
return new LiteralExpressionGridRow(getExpressionTextLineHeight(getRenderer().getTheme()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would prefer to keep this factory method (even with a simple new statement) so it will be easier to change in the future. Btw no strong opinion

}

private Supplier<List<GridColumn.HeaderMetaData>> outputClauseHeaderMetaData(final OutputClause oc) {
return () -> {
final List<GridColumn.HeaderMetaData> metaData = new ArrayList<>();
Expand Down Expand Up @@ -392,7 +389,7 @@ public void setName(final Name name) {
@Override
public void initialiseUiRows() {
getExpression().get().ifPresent(e -> {
e.getRule().forEach(r -> model.appendRow(makeDecisionTableRow()));
e.getRule().forEach(r -> model.appendRow(new LiteralExpressionGridRow()));
});
}

Expand Down Expand Up @@ -654,7 +651,7 @@ void deleteOutputClause(final int index) {

void addDecisionRule(final int index) {
getExpression().get().ifPresent(dtable -> {
final GridRow decisionTableRow = makeDecisionTableRow();
final GridRow decisionTableRow = new LiteralExpressionGridRow();
final DecisionRule decisionRule = DecisionRuleFactory.makeDecisionRule(dtable);
sessionCommandManager.execute((AbstractCanvasHandler) sessionManager.getCurrentSession().getCanvasHandler(),
new AddDecisionRuleCommand(dtable,
Expand All @@ -679,7 +676,7 @@ void deleteDecisionRule(final int index) {

void duplicateDecisionRule(final int index) {
getExpression().get().ifPresent(dtable -> {
final GridRow decisionTableRow = makeDecisionTableRow();
final GridRow decisionTableRow = new LiteralExpressionGridRow();
final DecisionRule decisionRule = DecisionRuleFactory.duplicateDecisionRule(index, dtable);
sessionCommandManager.execute((AbstractCanvasHandler) sessionManager.getCurrentSession().getCanvasHandler(),
new AddDecisionRuleCommand(dtable,
Expand Down
9 changes: 5 additions & 4 deletions ...e/workbench/common/dmn/client/editors/expressions/types/dtable/DecisionTableGridCell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
import java.util.Optional;

import org.kie.workbench.common.dmn.client.widgets.grid.controls.list.ListSelectorView;
import org.kie.workbench.common.dmn.client.widgets.grid.model.DMNGridCell;
import org.kie.workbench.common.dmn.client.widgets.grid.model.BaseHasDynamicHeightCell;
import org.uberfire.ext.wires.core.grids.client.model.GridCellValue;

public class DecisionTableGridCell<T> extends DMNGridCell<T> {
public class DecisionTableGridCell<T> extends BaseHasDynamicHeightCell<T> {

private final ListSelectorView.Presenter listSelector;

public DecisionTableGridCell(final GridCellValue<T> value,
final ListSelectorView.Presenter listSelector) {
super(value);
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(value, lineHeight);
this.listSelector = listSelector;
}

Expand Down
17 changes: 12 additions & 5 deletions ...kbench/common/dmn/client/editors/expressions/types/dtable/DecisionTableUIModelMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@
public class DecisionTableUIModelMapper extends BaseUIModelMapper<DecisionTable> {

private final ListSelectorView.Presenter listSelector;
private final double lineHeight;

public DecisionTableUIModelMapper(final Supplier<GridData> uiModel,
final Supplier<Optional<DecisionTable>> dmnModel,
final ListSelectorView.Presenter listSelector) {
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(uiModel,
dmnModel);
this.listSelector = listSelector;
this.lineHeight = lineHeight;
}

@Override
Expand All @@ -52,7 +55,8 @@ public void fromDMNModel(final int rowIndex,
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new DecisionTableGridCell<>(new BaseGridCellValue<>(rowIndex + 1),
listSelector));
listSelector,
lineHeight));
uiModel.get().getCell(rowIndex,
columnIndex).setSelectionStrategy(RowSelectionStrategy.INSTANCE);
break;
Expand All @@ -61,20 +65,23 @@ public void fromDMNModel(final int rowIndex,
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new DecisionTableGridCell<>(new BaseGridCellValue<>(rule.getInputEntry().get(iei).getText().getValue()),
listSelector));
listSelector,
lineHeight));
break;
case OUTPUT_CLAUSES:
final int oei = DecisionTableUIModelMapperHelper.getOutputEntryIndex(dtable, columnIndex);
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new DecisionTableGridCell<>(new BaseGridCellValue<>(rule.getOutputEntry().get(oei).getText().getValue()),
listSelector));
listSelector,
lineHeight));
break;
case DESCRIPTION:
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new DecisionTableGridCell<>(new BaseGridCellValue<>(rule.getDescription().getValue()),
listSelector));
listSelector,
lineHeight));
break;
}
});
Expand Down
3 changes: 3 additions & 0 deletions ...ient/editors/expressions/types/function/supplementary/pmml/LiteralExpressionPMMLGrid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
import org.uberfire.ext.wires.core.grids.client.widget.grid.NodeMouseEventHandler;
import org.uberfire.ext.wires.core.grids.client.widget.layer.GridSelectionManager;

import static org.kie.workbench.common.dmn.client.editors.expressions.util.RendererUtils.getExpressionTextLineHeight;

public abstract class LiteralExpressionPMMLGrid extends LiteralExpressionGrid {

public LiteralExpressionPMMLGrid(final GridCellTuple parent,
Expand Down Expand Up @@ -116,6 +118,7 @@ public LiteralExpressionUIModelMapper makeUiModelMapper() {
return new LiteralExpressionPMMLUIModelMapper(this::getModel,
getExpression(),
listSelector,
getExpressionTextLineHeight(getRenderer().getTheme()),
getPlaceHolder());
}

Expand Down
4 changes: 3 additions & 1 deletion ...ors/expressions/types/function/supplementary/pmml/LiteralExpressionPMMLUIModelMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ public class LiteralExpressionPMMLUIModelMapper extends LiteralExpressionUIModel
public LiteralExpressionPMMLUIModelMapper(final Supplier<GridData> uiModel,
final Supplier<Optional<LiteralExpression>> dmnModel,
final ListSelectorView.Presenter listSelector,
final double lineHeight,
final String placeHolder) {
super(uiModel,
dmnModel,
listSelector);
listSelector,
lineHeight);
this.placeHolder = placeHolder;
}

Expand Down
9 changes: 5 additions & 4 deletions .../workbench/common/dmn/client/editors/expressions/types/literal/LiteralExpressionCell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
import java.util.Optional;

import org.kie.workbench.common.dmn.client.widgets.grid.controls.list.ListSelectorView;
import org.kie.workbench.common.dmn.client.widgets.grid.model.DMNGridCell;
import org.kie.workbench.common.dmn.client.widgets.grid.model.BaseHasDynamicHeightCell;
import org.uberfire.ext.wires.core.grids.client.model.GridCellValue;

public class LiteralExpressionCell<T> extends DMNGridCell<T> {
public class LiteralExpressionCell<T> extends BaseHasDynamicHeightCell<T> {

private final ListSelectorView.Presenter listSelector;

public LiteralExpressionCell(final GridCellValue<T> value,
final ListSelectorView.Presenter listSelector) {
super(value);
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(value, lineHeight);
this.listSelector = listSelector;
}

Expand Down
5 changes: 3 additions & 2 deletions .../workbench/common/dmn/client/editors/expressions/types/literal/LiteralExpressionGrid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ protected void doInitialisation() {
public LiteralExpressionUIModelMapper makeUiModelMapper() {
return new LiteralExpressionUIModelMapper(this::getModel,
getExpression(),
listSelector);
listSelector,
getExpressionTextLineHeight(getRenderer().getTheme()));
}

@Override
Expand All @@ -166,7 +167,7 @@ protected void initialiseUiColumns() {

@Override
public void initialiseUiRows() {
getExpression().get().ifPresent(e -> model.appendRow(new LiteralExpressionGridRow(getExpressionTextLineHeight(getRenderer().getTheme()))));
getExpression().get().ifPresent(e -> model.appendRow(new LiteralExpressionGridRow()));
}

@Override
Expand Down
8 changes: 6 additions & 2 deletions ...h/common/dmn/client/editors/expressions/types/literal/LiteralExpressionUIModelMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@
public class LiteralExpressionUIModelMapper extends BaseUIModelMapper<LiteralExpression> {

private final ListSelectorView.Presenter listSelector;
private final double lineHeight;

public LiteralExpressionUIModelMapper(final Supplier<GridData> uiModel,
final Supplier<Optional<LiteralExpression>> dmnModel,
final ListSelectorView.Presenter listSelector) {
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(uiModel,
dmnModel);
this.listSelector = listSelector;
this.lineHeight = lineHeight;
}

@Override
Expand All @@ -45,7 +48,8 @@ public void fromDMNModel(final int rowIndex,
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new LiteralExpressionCell<>(new BaseGridCellValue<>(literalExpression.getText().getValue()),
listSelector));
listSelector,
lineHeight));
});
}

Expand Down
11 changes: 4 additions & 7 deletions .../org/kie/workbench/common/dmn/client/editors/expressions/types/relation/RelationGrid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ protected void doInitialisation() {
public RelationUIModelMapper makeUiModelMapper() {
return new RelationUIModelMapper(this::getModel,
getExpression(),
listSelector);
listSelector,
getExpressionTextLineHeight(getRenderer().getTheme()));
}

@Override
Expand Down Expand Up @@ -169,14 +170,10 @@ private RelationColumn makeRelationColumn(final int index,
return relationColumn;
}

private GridRow makeRelationRow() {
return new LiteralExpressionGridRow(getExpressionTextLineHeight(getRenderer().getTheme()));
}

@Override
public void initialiseUiRows() {
getExpression().get().ifPresent(e -> {
e.getRow().forEach(r -> model.appendRow(makeRelationRow()));
e.getRow().forEach(r -> model.appendRow(new LiteralExpressionGridRow()));
});
}

Expand Down Expand Up @@ -303,7 +300,7 @@ void deleteColumn(final int index) {

void addRow(final int index) {
getExpression().get().ifPresent(relation -> {
final GridRow relationRow = makeRelationRow();
final GridRow relationRow = new LiteralExpressionGridRow();
sessionCommandManager.execute((AbstractCanvasHandler) sessionManager.getCurrentSession().getCanvasHandler(),
new AddRelationRowCommand(relation,
new List(),
Expand Down
9 changes: 5 additions & 4 deletions .../kie/workbench/common/dmn/client/editors/expressions/types/relation/RelationGridCell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
import java.util.Optional;

import org.kie.workbench.common.dmn.client.widgets.grid.controls.list.ListSelectorView;
import org.kie.workbench.common.dmn.client.widgets.grid.model.DMNGridCell;
import org.kie.workbench.common.dmn.client.widgets.grid.model.BaseHasDynamicHeightCell;
import org.uberfire.ext.wires.core.grids.client.model.GridCellValue;

public class RelationGridCell<T> extends DMNGridCell<T> {
public class RelationGridCell<T> extends BaseHasDynamicHeightCell<T> {

private final ListSelectorView.Presenter listSelector;

public RelationGridCell(final GridCellValue<T> value,
final ListSelectorView.Presenter listSelector) {
super(value);
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(value, lineHeight);
this.listSelector = listSelector;
}

Expand Down
11 changes: 8 additions & 3 deletions ...workbench/common/dmn/client/editors/expressions/types/relation/RelationUIModelMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@
public class RelationUIModelMapper extends BaseUIModelMapper<Relation> {

private final ListSelectorView.Presenter listSelector;
private final double lineHeight;

public RelationUIModelMapper(final Supplier<GridData> uiModel,
final Supplier<Optional<Relation>> dmnModel,
final ListSelectorView.Presenter listSelector) {
final ListSelectorView.Presenter listSelector,
final double lineHeight) {
super(uiModel,
dmnModel);
this.listSelector = listSelector;
this.lineHeight = lineHeight;
}

@Override
Expand All @@ -51,7 +54,8 @@ public void fromDMNModel(final int rowIndex,
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new RelationGridCell<>(new BaseGridCellValue<>(rowIndex + 1),
listSelector));
listSelector,
lineHeight));
uiModel.get().getCell(rowIndex,
columnIndex).setSelectionStrategy(RowSelectionStrategy.INSTANCE);
break;
Expand All @@ -69,7 +73,8 @@ public void fromDMNModel(final int rowIndex,
uiModel.get().setCell(rowIndex,
columnIndex,
() -> new RelationGridCell<>(new BaseGridCellValue<>(le.getText().getValue()),
listSelector));
listSelector,
lineHeight));
});
}
});
Expand Down
3 changes: 3 additions & 0 deletions .../kie/workbench/common/dmn/client/widgets/dnd/DelegatingGridWidgetDndMouseMoveHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.uberfire.ext.wires.core.grids.client.widget.dnd.GridWidgetDnDHandlersState;
import org.uberfire.ext.wires.core.grids.client.widget.dnd.GridWidgetDnDMouseMoveHandler;
import org.uberfire.ext.wires.core.grids.client.widget.grid.GridWidget;
import org.uberfire.ext.wires.core.grids.client.widget.grid.renderers.grids.impl.BaseGridRendererHelper;
import org.uberfire.ext.wires.core.grids.client.widget.layer.GridLayer;

public class DelegatingGridWidgetDndMouseMoveHandler extends GridWidgetDnDMouseMoveHandler {
Expand All @@ -32,9 +33,11 @@ public DelegatingGridWidgetDndMouseMoveHandler(final GridLayer layer,

@Override
protected void findMovableRows(final GridWidget view,
final BaseGridRendererHelper.RenderingInformation renderingInformation,
final double cx,
final double cy) {
super.findMovableRows(view,
renderingInformation,
cx,
cy);
if (view instanceof HasRowDragRestrictions) {
Expand Down