Skip to content

Commit 3b9b44f

Browse files
HeikoKlareptziegler
authored andcommitted
Make DirectEditManager properly handle changed cell editor state
The DirectEditManager maintains an own dirty state to identify if a command has to be executed on commit. In case it has an underlying cell editor, this is when the cell editor submits its change. However, a dirty state is already tracked by the underlying cell editor in that case and the two are not necessarily consistent. The DirectEditManager only updates its dirty state on a `editorValueChanged` event from the cell editor, which is by definition only sent on textual input changes. In case a selection is made, such as in case of the ComboBoxCellEditor, no such `editorValueChanged` event is submitted but only the `applyEditorValue` event is triggered. In consequences, changes in ComboBoxCellEditors are currently not properly converted into according commands by the DirectEditManager. This change adapts the DirectEditManager to consider the dirty state of the underlying cell editor in addition to a potentially set custom dirty state on the DirectEditManager itself.
1 parent da3a6d6 commit 3b9b44f

File tree

3 files changed

+147
-6
lines changed

3 files changed

+147
-6
lines changed
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vector Informatik GmbH and others.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*******************************************************************************/
10+
package org.eclipse.gef.test;
11+
12+
import static org.junit.jupiter.api.Assertions.assertTrue;
13+
14+
import java.util.Collection;
15+
import java.util.concurrent.atomic.AtomicBoolean;
16+
17+
import org.eclipse.swt.widgets.Composite;
18+
import org.eclipse.swt.widgets.Control;
19+
import org.eclipse.swt.widgets.Shell;
20+
21+
import org.eclipse.jface.viewers.CellEditor;
22+
import org.eclipse.jface.viewers.ComboBoxCellEditor;
23+
import org.eclipse.ui.PlatformUI;
24+
25+
import org.eclipse.draw2d.Figure;
26+
import org.eclipse.draw2d.IFigure;
27+
import org.eclipse.draw2d.geometry.Point;
28+
29+
import org.eclipse.gef.EditDomain;
30+
import org.eclipse.gef.EditPart;
31+
import org.eclipse.gef.GraphicalEditPart;
32+
import org.eclipse.gef.Request;
33+
import org.eclipse.gef.commands.Command;
34+
import org.eclipse.gef.editparts.AbstractGraphicalEditPart;
35+
import org.eclipse.gef.tools.DirectEditManager;
36+
import org.eclipse.gef.ui.parts.AbstractEditPartViewer;
37+
38+
import org.junit.jupiter.api.Test;
39+
40+
public class DirectEditManagerTest {
41+
42+
@SuppressWarnings("static-method")
43+
@Test
44+
public void testComboCellEditorChangeProcessed() {
45+
Shell shell = PlatformUI.getWorkbench().getDisplay()
46+
.syncCall(() -> PlatformUI.getWorkbench().getDisplay().getActiveShell());
47+
EditDomain editDomain = new EditDomain();
48+
TestDirectEditManager editManager = new TestDirectEditManager(createDummyEditPart(shell, editDomain));
49+
AtomicBoolean comboChangeCommandSubmitted = new AtomicBoolean();
50+
editDomain.getCommandStack().addCommandStackEventListener(event -> comboChangeCommandSubmitted.set(true));
51+
52+
PlatformUI.getWorkbench().getDisplay().syncExec(() -> {
53+
editManager.show();
54+
// Change value of combo cell editor
55+
editManager.getCellEditor().setValue(0);
56+
// Make combo cell editor lose focus to apply its value
57+
shell.setFocus();
58+
});
59+
60+
assertTrue(comboChangeCommandSubmitted.get());
61+
}
62+
63+
private static class TestDirectEditManager extends DirectEditManager {
64+
public TestDirectEditManager(GraphicalEditPart source) {
65+
super(source, ComboBoxCellEditor.class, cellEditor -> {
66+
});
67+
}
68+
69+
@Override
70+
protected CellEditor createCellEditorOn(Composite composite) {
71+
return new ComboBoxCellEditor(composite, new String[] { "test" }); //$NON-NLS-1$
72+
}
73+
74+
@Override
75+
public CellEditor getCellEditor() {
76+
return super.getCellEditor();
77+
}
78+
79+
@Override
80+
public void showFeedback() {
81+
}
82+
83+
@Override
84+
protected void initCellEditor() {
85+
}
86+
}
87+
88+
private static GraphicalEditPart createDummyEditPart(Control control, EditDomain editDomain) {
89+
return new AbstractGraphicalEditPart() {
90+
@Override
91+
protected void createEditPolicies() {
92+
}
93+
94+
@Override
95+
public org.eclipse.gef.EditPartViewer getViewer() {
96+
return new AbstractEditPartViewer() {
97+
@Override
98+
public EditPart findObjectAtExcluding(Point location, Collection<IFigure> exclusionSet,
99+
Conditional conditional) {
100+
return null;
101+
}
102+
103+
@Override
104+
public Control createControl(Composite parent) {
105+
return parent;
106+
}
107+
108+
@Override
109+
public Control getControl() {
110+
return control;
111+
}
112+
113+
@Override
114+
public org.eclipse.gef.EditDomain getEditDomain() {
115+
return editDomain;
116+
}
117+
};
118+
}
119+
120+
@Override
121+
public Command getCommand(Request request) {
122+
return new Command() {
123+
};
124+
}
125+
126+
@Override
127+
protected IFigure createFigure() {
128+
return new Figure();
129+
}
130+
};
131+
}
132+
133+
}

org.eclipse.gef.tests/src/org/eclipse/gef/test/GEFTestSuite.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
RulerLayoutTests.class,
3232
GraphicalViewerTest.class,
3333
PaletteColorProviderTest.class,
34-
SWTBotTestSuite.class
34+
SWTBotTestSuite.class,
35+
DirectEditManagerTest.class
3536
})
3637
public class GEFTestSuite {
3738
}

org.eclipse.gef/src/org/eclipse/gef/tools/DirectEditManager.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public abstract class DirectEditManager {
6262
private IFigure cellEditorFrame;
6363
private ICellEditorListener cellEditorListener;
6464
private boolean showingFeedback;
65-
private boolean dirty;
65+
private boolean customDirtyState;
6666
private DirectEditRequest request;
6767
private CellEditorLocator locator;
6868
private GraphicalEditPart source;
@@ -121,7 +121,7 @@ protected void bringDown() {
121121
setCellEditor(null);
122122
}
123123
request = null;
124-
dirty = false;
124+
customDirtyState = false;
125125
}
126126

127127
/**
@@ -260,7 +260,10 @@ protected CellEditorLocator getLocator() {
260260
}
261261

262262
protected void handleValueChanged() {
263-
setDirty(true);
263+
CellEditor cellEditor = getCellEditor();
264+
if (cellEditor == null) {
265+
setDirty(true);
266+
}
264267
showFeedback();
265268
placeCellEditor();
266269
}
@@ -332,7 +335,11 @@ public void partDeactivated(EditPart editpart) {
332335
* @return <code>true</code> if the cell editor is dirty
333336
*/
334337
protected boolean isDirty() {
335-
return dirty;
338+
CellEditor cellEditor = getCellEditor();
339+
if (cellEditor == null) {
340+
return customDirtyState;
341+
}
342+
return cellEditor.isDirty();
336343
}
337344

338345
private void placeCellEditorFrame() {
@@ -368,7 +375,7 @@ protected void setCellEditor(CellEditor editor) {
368375
* @param value the dirty property
369376
*/
370377
protected void setDirty(boolean value) {
371-
dirty = value;
378+
customDirtyState = value;
372379
}
373380

374381
/**

0 commit comments

Comments
 (0)