Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

Commit

Permalink
move get/setCursor from Renderer/IRenderer/AtomContainerRenderer to C…
Browse files Browse the repository at this point in the history 
…ontrollerHub/IChemModelRelay; fixes issue JChemPaint#13
  • Loading branch information
@rwst
rwst committed May 28, 2012
1 parent 0e0128e commit c58ca2f
Show file tree
Hide file tree
Showing 6 changed files with 2,787 additions and 35 deletions.
25 changes: 20 additions & 5 deletions src/main/org/openscience/jchempaint/controller/ControllerHub.java
View file
Expand Up @@ -73,6 +73,7 @@
import org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator;
import org.openscience.cdk.tools.manipulator.ReactionManipulator;
import org.openscience.cdk.validate.ProblemMarker;
import org.openscience.jchempaint.RenderPanel;
import org.openscience.jchempaint.applet.JChemPaintAbstractApplet;
import org.openscience.jchempaint.controller.undoredo.IUndoRedoFactory;
import org.openscience.jchempaint.controller.undoredo.IUndoRedoable;
Expand Down Expand Up @@ -111,7 +112,7 @@ public class ControllerHub implements IMouseEventRelay, IChemModelRelay {

private IRenderer renderer;

private IViewEventRelay eventRelay;
private RenderPanel eventRelay;

This comment has been minimized.

Sign in to view

Copy link
@egonw

egonw Jun 23, 2012

In general it is better to write against interfaces than implementations. Also, the commit message suggests to me you could reuse an existing class, but then you would simply remove an interface, not?

Anyway, getCursor() should indeed not be part of the Renderer... so, I second very much fixing that.

This comment has been minimized.

Sign in to view

Copy link
@rwst

rwst Jun 23, 2012

Author Owner

The reason for using the implementation was it was easier to do than changing the interface. You may want to open an issue for this in the tracker.
private List<IControllerModule> generalModules;

Expand Down Expand Up @@ -146,7 +147,7 @@ public class ControllerHub implements IMouseEventRelay, IChemModelRelay {
private String phantomText = null;

public ControllerHub(IControllerModel controllerModel, IRenderer renderer,
IChemModel chemModel, IViewEventRelay eventRelay,
IChemModel chemModel, RenderPanel eventRelay,
UndoRedoHandler undoredohandler, IUndoRedoFactory undoredofactory,
boolean isViewer, JChemPaintAbstractApplet applet) {
this.controllerModel = controllerModel;
Expand Down Expand Up @@ -272,8 +273,8 @@ public void mouseClickedDown(int screenX, int screenY) {
if (activeModule != null)
activeModule.mouseClickedDown(modelCoord);

if (renderer.getCursor() == Cursor.HAND_CURSOR
|| renderer.getCursor() == Cursor.HAND_CURSOR) {
if (getCursor() == Cursor.HAND_CURSOR
|| getCursor() == Cursor.HAND_CURSOR) {
setCursor(Cursor.MOVE_CURSOR);
oldMouseCursor = Cursor.HAND_CURSOR;
} else {
Expand Down Expand Up @@ -2655,8 +2656,22 @@ public IChemModel getChemModel() {
return chemModel;
}

/**
* Sets the mouse cursor shown on the renderPanel.
*
* @param cursor One of the constants from java.awt.Cursor.
*/
public void setCursor(int cursor) {
renderer.setCursor(cursor);
eventRelay.setCursor(new Cursor(cursor));
}

/**
* Tells the mouse cursor shown on the renderPanel.
*
* @return One of the constants from java.awt.Cursor.
*/
public int getCursor() {
return eventRelay.getCursor().getType();
}

/**
Expand Down

5 comments on commit c58ca2f

@egonw
Copy link

@egonw egonw commented on c58ca2f Jun 23, 2012

Choose a reason for hiding this comment

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

Ralf, this is a patch that, when cleaned up should go into CDK/CDK-JChemPaint.

Do you think you can rebase it as patch on cdk-1.4.x?

@rwst
Copy link
Owner Author

@rwst rwst commented on c58ca2f Jun 23, 2012

Choose a reason for hiding this comment

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

What patch? The last 4 lines? You cannot mean the whole commit because it undoes something that is only in JCP. Please be more specific.

@egonw
Copy link

@egonw egonw commented on c58ca2f Jun 25, 2012

Choose a reason for hiding this comment

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

No, the full patch (minus that .orig file): c58ca2f

I am sorry... This GitHub review website is not too intuitive...

@rwst
Copy link
Owner Author

@rwst rwst commented on c58ca2f Jun 25, 2012

Choose a reason for hiding this comment

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

As this patch 1. removes something from Renderer that is only in JCP, and 2. adds something to ControllerHub. You can only mean 2. add getCursor functionality to ControllerHub in CDK. But I still haven't agreed to backport anything to CDK controller. Remember that I first wanted to have an overview to decide which parts of controller belong to JCP vs. CDK. This principle decision hasn't been made (and probably won't for some time), so I won't know if I'll backport that patch until then.

@egonw
Copy link

@egonw egonw commented on c58ca2f Jun 25, 2012

Choose a reason for hiding this comment

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

OK, understood :)

Please sign in to comment.