search

piccolo2d / piccolo2d.java

Structured 2D Graphics Framework
http://piccolo2d.org
Other
51 stars 14 forks source link

Refactor PNode.moveToBack() and related #166

Closed mro closed 9 years ago

mro commented 9 years ago

Originally reported on Google Code with ID 166

I would like to rename & refactor the Z-order methods in PNode to the following

Methods that change the Z-order of this

void raise();
void lower();
void raiseToTop();
void lowerToBottom();

Methods that change the Z-order of a child node

void raise(PNode child)
void lower(PNode child)
void raiseToTop(PNode child)
void lowerToBottom(PNode child)

I find the current methods, as defined in terms of their parent, rather
awkwardly defined as evidenced by Issue 165.

These new method names are the action names used in Inkscape.

Reported by heuermh on 2010-02-26 17:57:14

mro commented 9 years ago 
Unless you keep the existing methods doesn't moving a child to be immediately before
one of its siblings cumbersome?

Reported by allain.lalonde on 2010-02-26 20:33:25

mro commented 9 years ago 
This issue was closed by revision r974.

Reported by heuermh on 2010-02-26 21:55:47

mro commented 9 years ago 
False alarm, issue 165 was fixed by r974, not this one.

Reported by heuermh on 2010-02-26 22:01:42

mro commented 9 years ago 
raiseToTop() == moveToFront()
lowerToBack() == moveToBack()

But I don't understand how the 4 raise and lower methods work -- raise and lower
relative to what? Perhaps this is a reason to keep the existing methods.  I also like
the fact that all of the existing method names start with "move", much easier to
remember.  So I'm not sold on this change.

Reported by cmalley@pixelzoom.com on 2010-02-27 18:20:09

mro commented 9 years ago 
Copying this comment from a thread on piccolo2d-dev@ to here

cmalley@pixelzoom.com wrote:

"I realize this is z-ordering.  But if this is supposed to replace the
existing z-ordering interface, then raise and lower would be replacing
moveToFrontOf(PNode) and moveInBackOf(PNode).  So, again, how does
raise and lower allow me to put one sibling in front/back of another
sibling?  Imho, this new interface proposal reduces the functionality
of the interface, and (as Allain noted previously) makes make changing
the z-order of siblings cumbersome, if not downright difficult."

Reported by heuermh on 2010-04-09 21:31:07

mro commented 9 years ago 
Methods that change the Z-order of a peer node

void raiseAbove(PNode peer)
void lowerBelow(PNode peer)

?

Reported by heuermh on 2010-04-09 22:10:18

mro commented 9 years ago 
My revised proposal is as follows:

Methods that change the Z-order of this

void raise();
void lower();
void raiseToTop();
void lowerToBottom();
void raiseAbove(PNode peer);
void lowerBelow(PNode peer);

Methods that change the Z-order of a child node

void raise(PNode child)
void lower(PNode child)
void raiseToTop(PNode child)
void lowerToBottom(PNode child)

Reported by heuermh on 2011-11-01 19:27:40

mro commented 9 years ago

Reported by heuermh on 2013-11-26 21:57:50

mro commented 9 years ago

Reported by heuermh on 2013-11-26 21:58:06

mro commented 9 years ago 
$ svn commit -m "Issue 166 ; refactor PNode moveToBack and related" .
Sending        core/src/main/java/org/piccolo2d/PNode.java
Sending        core/src/main/java/org/piccolo2d/event/PDragEventHandler.java
Sending        core/src/test/java/org/piccolo2d/PNodeTest.java
Sending        core/src/test/java/org/piccolo2d/event/PDragEventHandlerTest.java
Sending        examples/src/main/java/org/piccolo2d/examples/GridExample.java
Sending        examples/src/main/java/org/piccolo2d/tutorial/PiccoloPresentation.java
Transmitting file data ......
Committed revision 1227.

Reported by heuermh on 2013-11-26 22:55:02

mro commented 9 years ago 
$ svn commit -m "Issue 166 ; mark PNode moveToFront and related as @deprecated" .
Sending        core/src/main/java/org/piccolo2d/PNode.java
Sending        core/src/main/java/org/piccolo2d/event/PDragEventHandler.java
Transmitting file data ..
Committed revision 1228.

Reported by heuermh on 2013-11-26 22:59:24

mro commented 9 years ago

Reported by heuermh on 2013-11-26 23:18:55

heuermh commented 9 years ago

Fixed by commit f6b9d506cb2e9d266ce120aee7fe5455de6299c2 and commit 54bd1dad9f3b51cf1dc4d70bfedf093cce36fcae