The Trunk: Morphic-cmm.1443.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Morphic-cmm.1443.mcz

commits-2
Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1443.mcz

==================== Summary ====================

Name: Morphic-cmm.1443
Author: cmm
Time: 30 May 2018, 4:50:16.716113 pm
UUID: 2f269941-1afa-4615-9242-9e485ecd986e
Ancestors: Morphic-mt.1442

- When updating a borderStyle:, only signal changes if the receiver is in the world for them to be seen.  This fixes:
        NCAAConnectorMorph new
- When halo's are on a background Morph, and a foreground morph covering it is blue-clicked, transfer the halos to the clicked morph.
- Disable the new #balanceOffsets when Smart Splitters is enabled, unfortunately they currently don't play nicely together.

=============== Diff against Morphic-mt.1442 ===============

Item was changed:
  ----- Method: Morph>>borderStyle: (in category 'accessing') -----
  borderStyle: aBorderStyle
-
  aBorderStyle = self borderStyle ifTrue: [^ self].
-
  "If we cannot draw the new border, accept at least its color and width."
  ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
  ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
  ifFalse: [
  self borderStyle
  width: aBorderStyle width;
  baseColor: aBorderStyle baseColor].
-
  self borderStyle trackColorFrom: self.
+ self isInWorld ifTrue:
+ [ self
+ layoutChanged;
+ changed ]!
-
- self
- layoutChanged;
- changed.!

Item was changed:
  ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing') -----
+ tryInvokeHalo: aUserInputEvent
+ "Invoke halos around the top-most world container at aUserInputEvent's #position.  If it was already halo'd, zero-in on its next inward component morph at that position.  Holding Shift during the click reverses this traversal order."
+ | stack target owners |
+ Preferences noviceMode ifTrue: [^self ].
+ Morph haloForAll ifFalse: [^self].
+ "the stack is the top-most morph to bottom-most."
+ stack := (self morphsAt: aUserInputEvent position unlocked: true) select: [ : each | each wantsHaloFromClick ].
+ stack ifEmpty: [^self].
+ target :=
+ aUserInputEvent hand halo
+ ifNil: [ stack first ]
+ ifNotNil:
+ [ : existingHalo |
+ stack allButFirst "halo's are always topmost on the stack"
+ detect: [ : each | each owner == self ]
+ ifFound:
+ [ : topMostWhereClicked |
+ (existingHalo target withAllOwners includes: topMostWhereClicked)
+ ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self ]
+ ifFalse:
+ [ "Transfer halo to new world container."
+ aUserInputEvent hand removeHalo.
+ aUserInputEvent shiftPressed
+ ifTrue: [ stack second ]
+ ifFalse: [ topMostWhereClicked ] ]  ]
+ ifNone: ["Okay, invoke halos on the world." self ]  ].
+ "With a modifier, we want the innermost, otherwise the outermost."
+ owners := target withAllOwners select: [ : each | each wantsHaloFromClick ].
+ "the last owner is expected to be self."
+ target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [ owners at: owners size-1 ifAbsent: [self] ].
+ "Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
+ aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
+ target invokeHaloOrMove: aUserInputEvent.
+ "aUserInputEvent has been consumed, don't let it cause any further side-effects."
+ aUserInputEvent ignore!
- tryInvokeHalo: anEvent
-
- | innerMost target |
- anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will handle transfer itself."].
- Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
- Morph haloForAll ifFalse: [^ self].
-
- innerMost := (self morphsAt: anEvent position unlocked: true) first.
-
- "1) Try to use innermost morph but skip all the ones that do not want to show a halo along the owner chain."
- target := innerMost.
- [target isNil or: [target wantsHaloFromClick]]
- whileFalse: [target := target owner].
- target ifNil: [^ self].
-
- "2) Without a modifier, which is normal, find the outermost container for that inner morph."
- (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
- | previousTargets |
- previousTargets := OrderedCollection new.
- [target notNil and: [target owner ~~ self]] whileTrue: [
- previousTargets add: target.
- target := target owner].
- target ifNil: [^ self].
- [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
- target := previousTargets removeLast].
- target wantsHaloFromClick ifFalse: [^ self]].
-
- "3) Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
- anEvent hand newMouseFocus: target event: anEvent.
- target invokeHaloOrMove: anEvent.
- anEvent ignore.!

Item was changed:
  ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events') -----
  mouseUp: anEvent
+ (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand showTemporaryCursor: nil ].
+ self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
+ traceMorph ifNotNil:
+ [ traceMorph delete.
+ traceMorph := nil ].
- (self bounds containsPoint: anEvent cursorPoint)
- ifFalse: [anEvent hand showTemporaryCursor: nil].
- self class fastSplitterResize
- ifTrue: [self updateFromEvent: anEvent].
- traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
  self color: self getOldColor.
+ "balanceOffsets currently disrupts Smart Splitter behavior."
+ (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self balanceOffsets ]!
-
- self balanceOffsets!

Item was changed:
  ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
  step
  splitsTopAndBottom
  ifTrue: [ self reduceTopBottomImbalance ]
  ifFalse:
  [ self reduceLeftRightImbalance abs > 1 ifTrue:
  [ self splittersLeftDo:
  [ : splitter | splitter reduceLeftRightImbalance ].
  self splittersRightDo:
+ [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
- [ : splitter | splitter reduceLeftRightImbalance ] ] ].
- self balanceOffsets!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
Hey Chris,

both #layoutChanged and #changed MUST work for all morphs even if the morph is not in a world or has no owner. Please revert this change and fix NCAAConnectorMorph instad. Otherwise, this could break test code easily.

Best,
Marcel

Am 30.05.2018 23:51:51 schrieb [hidden email] <[hidden email]>:

Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1443.mcz

==================== Summary ====================

Name: Morphic-cmm.1443
Author: cmm
Time: 30 May 2018, 4:50:16.716113 pm
UUID: 2f269941-1afa-4615-9242-9e485ecd986e
Ancestors: Morphic-mt.1442

- When updating a borderStyle:, only signal changes if the receiver is in the world for them to be seen. This fixes:
NCAAConnectorMorph new
- When halo's are on a background Morph, and a foreground morph covering it is blue-clicked, transfer the halos to the clicked morph.
- Disable the new #balanceOffsets when Smart Splitters is enabled, unfortunately they currently don't play nicely together.

=============== Diff against Morphic-mt.1442 ===============

Item was changed:
----- Method: Morph>>borderStyle: (in category 'accessing') -----
borderStyle: aBorderStyle
-
aBorderStyle = self borderStyle ifTrue: [^ self].
-
"If we cannot draw the new border, accept at least its color and width."
((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
ifFalse: [
self borderStyle
width: aBorderStyle width;
baseColor: aBorderStyle baseColor].
-
self borderStyle trackColorFrom: self.
+ self isInWorld ifTrue:
+ [ self
+ layoutChanged;
+ changed ]!
-
- self
- layoutChanged;
- changed.!

Item was changed:
----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing') -----
+ tryInvokeHalo: aUserInputEvent
+ "Invoke halos around the top-most world container at aUserInputEvent's #position. If it was already halo'd, zero-in on its next inward component morph at that position. Holding Shift during the click reverses this traversal order."
+ | stack target owners |
+ Preferences noviceMode ifTrue: [^self ].
+ Morph haloForAll ifFalse: [^self].
+ "the stack is the top-most morph to bottom-most."
+ stack := (self morphsAt: aUserInputEvent position unlocked: true) select: [ : each | each wantsHaloFromClick ].
+ stack ifEmpty: [^self].
+ target :=
+ aUserInputEvent hand halo
+ ifNil: [ stack first ]
+ ifNotNil:
+ [ : existingHalo |
+ stack allButFirst "halo's are always topmost on the stack"
+ detect: [ : each | each owner == self ]
+ ifFound:
+ [ : topMostWhereClicked |
+ (existingHalo target withAllOwners includes: topMostWhereClicked)
+ ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self ]
+ ifFalse:
+ [ "Transfer halo to new world container."
+ aUserInputEvent hand removeHalo.
+ aUserInputEvent shiftPressed
+ ifTrue: [ stack second ]
+ ifFalse: [ topMostWhereClicked ] ] ]
+ ifNone: ["Okay, invoke halos on the world." self ] ].
+ "With a modifier, we want the innermost, otherwise the outermost."
+ owners := target withAllOwners select: [ : each | each wantsHaloFromClick ].
+ "the last owner is expected to be self."
+ target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [ owners at: owners size-1 ifAbsent: [self] ].
+ "Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
+ aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
+ target invokeHaloOrMove: aUserInputEvent.
+ "aUserInputEvent has been consumed, don't let it cause any further side-effects."
+ aUserInputEvent ignore!
- tryInvokeHalo: anEvent
-
- | innerMost target |
- anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will handle transfer itself."].
- Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
- Morph haloForAll ifFalse: [^ self].
-
- innerMost := (self morphsAt: anEvent position unlocked: true) first.
-
- "1) Try to use innermost morph but skip all the ones that do not want to show a halo along the owner chain."
- target := innerMost.
- [target isNil or: [target wantsHaloFromClick]]
- whileFalse: [target := target owner].
- target ifNil: [^ self].
-
- "2) Without a modifier, which is normal, find the outermost container for that inner morph."
- (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
- | previousTargets |
- previousTargets := OrderedCollection new.
- [target notNil and: [target owner ~~ self]] whileTrue: [
- previousTargets add: target.
- target := target owner].
- target ifNil: [^ self].
- [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
- target := previousTargets removeLast].
- target wantsHaloFromClick ifFalse: [^ self]].
-
- "3) Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
- anEvent hand newMouseFocus: target event: anEvent.
- target invokeHaloOrMove: anEvent.
- anEvent ignore.!

Item was changed:
----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events') -----
mouseUp: anEvent
+ (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand showTemporaryCursor: nil ].
+ self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
+ traceMorph ifNotNil:
+ [ traceMorph delete.
+ traceMorph := nil ].
- (self bounds containsPoint: anEvent cursorPoint)
- ifFalse: [anEvent hand showTemporaryCursor: nil].
- self class fastSplitterResize
- ifTrue: [self updateFromEvent: anEvent].
- traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
self color: self getOldColor.
+ "balanceOffsets currently disrupts Smart Splitter behavior."
+ (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self balanceOffsets ]!
-
- self balanceOffsets!

Item was changed:
----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
step
splitsTopAndBottom
ifTrue: [ self reduceTopBottomImbalance ]
ifFalse:
[ self reduceLeftRightImbalance abs > 1 ifTrue:
[ self splittersLeftDo:
[ : splitter | splitter reduceLeftRightImbalance ].
self splittersRightDo:
+ [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
- [ : splitter | splitter reduceLeftRightImbalance ] ] ].
- self balanceOffsets!




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

Hannes Hirzel
Hi Chris

Are you working on getting the Connectors [1] add-on to work  fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel <[hidden email]> wrote:

> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> <[hidden email]>:
> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/

You changed ALL the formatting, making it close to impossible to make out what you actually changed. 

Before:


After:


This is bad. :-(

 You might have fixed a bug, which one? Maybe I can help here.

Best,
Marcel

Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:

Hi Chris

Are you working on getting the Connectors [1] add-on to work fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel wrote:

> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> :
> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
Hey Chris,

I found out what you changed. Yet, that behavior has been there since ages:


If you keep on clicking BLUE, you will not transfer the halo the overlapping system window but stay in the background one.

So, what are the reasons for changing this? If you want to walk down a different hierarchy, just do a RED click in between and then point to that front window. I have a feeling that changing this behavior makes it difficult to navigate halos in (partially) hidden morphs...

Best,
Marcel

Am 31.05.2018 09:09:42 schrieb Marcel Taeumel <[hidden email]>:

Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/

You changed ALL the formatting, making it close to impossible to make out what you actually changed. 

Before:


After:


This is bad. :-(

 You might have fixed a bug, which one? Maybe I can help here.

Best,
Marcel

Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:

Hi Chris

Are you working on getting the Connectors [1] add-on to work fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel wrote:

> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> :
> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
Hey Chris,

I spoke to some colleagues they agreed that your improvement in halo navigation is reasonable and more user-friendly. :-) Yet, I refactored your change so that the "sibling lookup" is in SimpleHaloMorph >> #transferHalo:. I hope you agree. See Morphic-mt.1444.

Best,
Marcel

Am 31.05.2018 09:48:27 schrieb Marcel Taeumel <[hidden email]>:

Hey Chris,

I found out what you changed. Yet, that behavior has been there since ages:


If you keep on clicking BLUE, you will not transfer the halo the overlapping system window but stay in the background one.

So, what are the reasons for changing this? If you want to walk down a different hierarchy, just do a RED click in between and then point to that front window. I have a feeling that changing this behavior makes it difficult to navigate halos in (partially) hidden morphs...

Best,
Marcel

Am 31.05.2018 09:09:42 schrieb Marcel Taeumel <[hidden email]>:

Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/

You changed ALL the formatting, making it close to impossible to make out what you actually changed. 

Before:


After:


This is bad. :-(

 You might have fixed a bug, which one? Maybe I can help here.

Best,
Marcel

Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:

Hi Chris

Are you working on getting the Connectors [1] add-on to work fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel wrote:

> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> :
> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
...I was referring to Morphic-mt.1445... Sorry...

Am 31.05.2018 10:35:01 schrieb Marcel Taeumel <[hidden email]>:

Hey Chris,

I spoke to some colleagues they agreed that your improvement in halo navigation is reasonable and more user-friendly. :-) Yet, I refactored your change so that the "sibling lookup" is in SimpleHaloMorph >> #transferHalo:. I hope you agree. See Morphic-mt.1444.

Best,
Marcel

Am 31.05.2018 09:48:27 schrieb Marcel Taeumel <[hidden email]>:

Hey Chris,

I found out what you changed. Yet, that behavior has been there since ages:


If you keep on clicking BLUE, you will not transfer the halo the overlapping system window but stay in the background one.

So, what are the reasons for changing this? If you want to walk down a different hierarchy, just do a RED click in between and then point to that front window. I have a feeling that changing this behavior makes it difficult to navigate halos in (partially) hidden morphs...

Best,
Marcel

Am 31.05.2018 09:09:42 schrieb Marcel Taeumel <[hidden email]>:

Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/

You changed ALL the formatting, making it close to impossible to make out what you actually changed. 

Before:


After:


This is bad. :-(

 You might have fixed a bug, which one? Maybe I can help here.

Best,
Marcel

Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:

Hi Chris

Are you working on getting the Connectors [1] add-on to work fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel wrote:

> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> :
> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

marcel.taeumel
In reply to this post by marcel.taeumel
Hey Chris,

I committed two changes to Connectors and FSM to address the issues for 5.2alpha:

Best,
Marcel

Am 31.05.2018 07:21:18 schrieb Marcel Taeumel <[hidden email]>:

Hey Chris,

both #layoutChanged and #changed MUST work for all morphs even if the morph is not in a world or has no owner. Please revert this change and fix NCAAConnectorMorph instad. Otherwise, this could break test code easily.

Best,
Marcel

Am 30.05.2018 23:51:51 schrieb [hidden email] <[hidden email]>:

Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1443.mcz

==================== Summary ====================

Name: Morphic-cmm.1443
Author: cmm
Time: 30 May 2018, 4:50:16.716113 pm
UUID: 2f269941-1afa-4615-9242-9e485ecd986e
Ancestors: Morphic-mt.1442

- When updating a borderStyle:, only signal changes if the receiver is in the world for them to be seen. This fixes:
NCAAConnectorMorph new
- When halo's are on a background Morph, and a foreground morph covering it is blue-clicked, transfer the halos to the clicked morph.
- Disable the new #balanceOffsets when Smart Splitters is enabled, unfortunately they currently don't play nicely together.

=============== Diff against Morphic-mt.1442 ===============

Item was changed:
----- Method: Morph>>borderStyle: (in category 'accessing') -----
borderStyle: aBorderStyle
-
aBorderStyle = self borderStyle ifTrue: [^ self].
-
"If we cannot draw the new border, accept at least its color and width."
((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
ifFalse: [
self borderStyle
width: aBorderStyle width;
baseColor: aBorderStyle baseColor].
-
self borderStyle trackColorFrom: self.
+ self isInWorld ifTrue:
+ [ self
+ layoutChanged;
+ changed ]!
-
- self
- layoutChanged;
- changed.!

Item was changed:
----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing') -----
+ tryInvokeHalo: aUserInputEvent
+ "Invoke halos around the top-most world container at aUserInputEvent's #position. If it was already halo'd, zero-in on its next inward component morph at that position. Holding Shift during the click reverses this traversal order."
+ | stack target owners |
+ Preferences noviceMode ifTrue: [^self ].
+ Morph haloForAll ifFalse: [^self].
+ "the stack is the top-most morph to bottom-most."
+ stack := (self morphsAt: aUserInputEvent position unlocked: true) select: [ : each | each wantsHaloFromClick ].
+ stack ifEmpty: [^self].
+ target :=
+ aUserInputEvent hand halo
+ ifNil: [ stack first ]
+ ifNotNil:
+ [ : existingHalo |
+ stack allButFirst "halo's are always topmost on the stack"
+ detect: [ : each | each owner == self ]
+ ifFound:
+ [ : topMostWhereClicked |
+ (existingHalo target withAllOwners includes: topMostWhereClicked)
+ ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self ]
+ ifFalse:
+ [ "Transfer halo to new world container."
+ aUserInputEvent hand removeHalo.
+ aUserInputEvent shiftPressed
+ ifTrue: [ stack second ]
+ ifFalse: [ topMostWhereClicked ] ] ]
+ ifNone: ["Okay, invoke halos on the world." self ] ].
+ "With a modifier, we want the innermost, otherwise the outermost."
+ owners := target withAllOwners select: [ : each | each wantsHaloFromClick ].
+ "the last owner is expected to be self."
+ target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [ owners at: owners size-1 ifAbsent: [self] ].
+ "Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
+ aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
+ target invokeHaloOrMove: aUserInputEvent.
+ "aUserInputEvent has been consumed, don't let it cause any further side-effects."
+ aUserInputEvent ignore!
- tryInvokeHalo: anEvent
-
- | innerMost target |
- anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will handle transfer itself."].
- Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
- Morph haloForAll ifFalse: [^ self].
-
- innerMost := (self morphsAt: anEvent position unlocked: true) first.
-
- "1) Try to use innermost morph but skip all the ones that do not want to show a halo along the owner chain."
- target := innerMost.
- [target isNil or: [target wantsHaloFromClick]]
- whileFalse: [target := target owner].
- target ifNil: [^ self].
-
- "2) Without a modifier, which is normal, find the outermost container for that inner morph."
- (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
- | previousTargets |
- previousTargets := OrderedCollection new.
- [target notNil and: [target owner ~~ self]] whileTrue: [
- previousTargets add: target.
- target := target owner].
- target ifNil: [^ self].
- [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
- target := previousTargets removeLast].
- target wantsHaloFromClick ifFalse: [^ self]].
-
- "3) Now that we have the target, show the halo. Abort event dispatching, too, to avoid confusion."
- anEvent hand newMouseFocus: target event: anEvent.
- target invokeHaloOrMove: anEvent.
- anEvent ignore.!

Item was changed:
----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events') -----
mouseUp: anEvent
+ (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand showTemporaryCursor: nil ].
+ self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
+ traceMorph ifNotNil:
+ [ traceMorph delete.
+ traceMorph := nil ].
- (self bounds containsPoint: anEvent cursorPoint)
- ifFalse: [anEvent hand showTemporaryCursor: nil].
- self class fastSplitterResize
- ifTrue: [self updateFromEvent: anEvent].
- traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
self color: self getOldColor.
+ "balanceOffsets currently disrupts Smart Splitter behavior."
+ (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self balanceOffsets ]!
-
- self balanceOffsets!

Item was changed:
----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
step
splitsTopAndBottom
ifTrue: [ self reduceTopBottomImbalance ]
ifFalse:
[ self reduceLeftRightImbalance abs > 1 ifTrue:
[ self splittersLeftDo:
[ : splitter | splitter reduceLeftRightImbalance ].
self splittersRightDo:
+ [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
- [ : splitter | splitter reduceLeftRightImbalance ] ] ].
- self balanceOffsets!




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

Chris Muller-3
In reply to this post by marcel.taeumel
Hi Marcel,

(Note:  I did not see this thread until after I sent the other private email).

You can't tell what I "changed" because the whole method was re-written.  The formatting I use is referred to as "Rectangular Block" and is a documented Smalltalk best-practice since the 1990's you can read about in Kent Beck's, "Smalltalk Best Practice Patterns".  Calling it "bad" is simply your opinion, bringing back the old confusing procedural code did break one case of functionality -- when a Morph is blue-clicked when the Shift modifier is pressed, it should select the innermost morph, *regardless* whether another morph has halos or not.

Please fix that.  I would recommend simply reverting back to my method, which is 10X easier to read and understand, but its your call.  I care more about the _functionality_ than the code style, but I do think my effort brought a significant improvement to that method.

Best,
  Chris

On Thu, May 31, 2018 at 2:09 AM, Marcel Taeumel <[hidden email]> wrote:
Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/

You changed ALL the formatting, making it close to impossible to make out what you actually changed. 

Before:


After:


This is bad. :-(

 You might have fixed a bug, which one? Maybe I can help here.

Best,
Marcel

Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:

Hi Chris

Are you working on getting the Connectors [1] add-on to work fine
with Squeak 5.2a?
Recently I loaded Connectors into Squeak6.0a and it loaded fine.
However I did not do any tests yet.

--Hannes

[1] Connectors
Connectors is a package by Ned Konz that turns Morphic into a drawing
environment for making connected drawings.
...
This project has been ported to Squeak 4.x by Chris Muller, and the
updated code is available at SqueakSource

Current and old versions are available at SqueakMap!



On 5/31/18, Marcel Taeumel wrote:
> Hey Chris,
>
> both #layoutChanged and #changed MUST work for all morphs even if the morph
> is not in a world or has no owner. Please revert this change and fix
> NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>
> Best,
> Marcel
> Am 30.05.2018 23:51:51 schrieb [hidden email]
> :

> Chris Muller uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-cmm.1443
> Author: cmm
> Time: 30 May 2018, 4:50:16.716113 pm
> UUID: 2f269941-1afa-4615-9242-9e485ecd986e
> Ancestors: Morphic-mt.1442
>
> - When updating a borderStyle:, only signal changes if the receiver is in
> the world for them to be seen. This fixes:
> NCAAConnectorMorph new
> - When halo's are on a background Morph, and a foreground morph covering it
> is blue-clicked, transfer the halos to the clicked morph.
> - Disable the new #balanceOffsets when Smart Splitters is enabled,
> unfortunately they currently don't play nicely together.
>
> =============== Diff against Morphic-mt.1442 ===============
>
> Item was changed:
> ----- Method: Morph>>borderStyle: (in category 'accessing') -----
> borderStyle: aBorderStyle
> -
> aBorderStyle = self borderStyle ifTrue: [^ self].
> -
> "If we cannot draw the new border, accept at least its color and width."
> ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
> ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
> ifFalse: [
> self borderStyle
> width: aBorderStyle width;
> baseColor: aBorderStyle baseColor].
> -
> self borderStyle trackColorFrom: self.
> + self isInWorld ifTrue:
> + [ self
> + layoutChanged;
> + changed ]!
> -
> - self
> - layoutChanged;
> - changed.!
>
> Item was changed:
> ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
> -----
> + tryInvokeHalo: aUserInputEvent
> + "Invoke halos around the top-most world container at aUserInputEvent's
> #position. If it was already halo'd, zero-in on its next inward component
> morph at that position. Holding Shift during the click reverses this
> traversal order."
> + | stack target owners |
> + Preferences noviceMode ifTrue: [^self ].
> + Morph haloForAll ifFalse: [^self].
> + "the stack is the top-most morph to bottom-most."
> + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
> [ : each | each wantsHaloFromClick ].
> + stack ifEmpty: [^self].
> + target :=
> + aUserInputEvent hand halo
> + ifNil: [ stack first ]
> + ifNotNil:
> + [ : existingHalo |
> + stack allButFirst "halo's are always topmost on the stack"
> + detect: [ : each | each owner == self ]
> + ifFound:
> + [ : topMostWhereClicked |
> + (existingHalo target withAllOwners includes: topMostWhereClicked)
> + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
> ]
> + ifFalse:
> + [ "Transfer halo to new world container."
> + aUserInputEvent hand removeHalo.
> + aUserInputEvent shiftPressed
> + ifTrue: [ stack second ]
> + ifFalse: [ topMostWhereClicked ] ] ]
> + ifNone: ["Okay, invoke halos on the world." self ] ].
> + "With a modifier, we want the innermost, otherwise the outermost."
> + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
> ].
> + "the last owner is expected to be self."
> + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
> owners at: owners size-1 ifAbsent: [self] ].
> + "Now that we have the target, show the halo. Abort event dispatching, too,
> to avoid confusion."
> + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
> + target invokeHaloOrMove: aUserInputEvent.
> + "aUserInputEvent has been consumed, don't let it cause any further
> side-effects."
> + aUserInputEvent ignore!
> - tryInvokeHalo: anEvent
> -
> - | innerMost target |
> - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
> handle transfer itself."].
> - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
> - Morph haloForAll ifFalse: [^ self].
> -
> - innerMost := (self morphsAt: anEvent position unlocked: true) first.
> -
> - "1) Try to use innermost morph but skip all the ones that do not want to
> show a halo along the owner chain."
> - target := innerMost.
> - [target isNil or: [target wantsHaloFromClick]]
> - whileFalse: [target := target owner].
> - target ifNil: [^ self].
> -
> - "2) Without a modifier, which is normal, find the outermost container for
> that inner morph."
> - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
> - | previousTargets |
> - previousTargets := OrderedCollection new.
> - [target notNil and: [target owner ~~ self]] whileTrue: [
> - previousTargets add: target.
> - target := target owner].
> - target ifNil: [^ self].
> - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
> - target := previousTargets removeLast].
> - target wantsHaloFromClick ifFalse: [^ self]].
> -
> - "3) Now that we have the target, show the halo. Abort event dispatching,
> too, to avoid confusion."
> - anEvent hand newMouseFocus: target event: anEvent.
> - target invokeHaloOrMove: anEvent.
> - anEvent ignore.!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
> -----
> mouseUp: anEvent
> + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
> showTemporaryCursor: nil ].
> + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
> + traceMorph ifNotNil:
> + [ traceMorph delete.
> + traceMorph := nil ].
> - (self bounds containsPoint: anEvent cursorPoint)
> - ifFalse: [anEvent hand showTemporaryCursor: nil].
> - self class fastSplitterResize
> - ifTrue: [self updateFromEvent: anEvent].
> - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
> self color: self getOldColor.
> + "balanceOffsets currently disrupts Smart Splitter behavior."
> + (ProportionalSplitterMorph smartVerticalSplitters or: [
> ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
> balanceOffsets ]!
> -
> - self balanceOffsets!
>
> Item was changed:
> ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
> step
> splitsTopAndBottom
> ifTrue: [ self reduceTopBottomImbalance ]
> ifFalse:
> [ self reduceLeftRightImbalance abs > 1 ifTrue:
> [ self splittersLeftDo:
> [ : splitter | splitter reduceLeftRightImbalance ].
> self splittersRightDo:
> + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
> - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
> - self balanceOffsets!
>
>
>







Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1443.mcz

Tobias Pape

> On 31.05.2018, at 19:32, Chris Muller <[hidden email]> wrote:
>
> Hi Marcel,
>
> (Note:  I did not see this thread until after I sent the other private email).
>
> You can't tell what I "changed" because the whole method was re-written.  The formatting I use is referred to as "Rectangular Block" and is a documented Smalltalk best-practice since the 1990's you can read about in Kent Beck's, "Smalltalk Best Practice Patterns".

Rectangular style is one thing, but another thing is having deep control flow nestings.

IMHO, that Method should actually be three  methods.

-t

>  Calling it "bad" is simply your opinion, bringing back the old confusing procedural code did break one case of functionality -- when a Morph is blue-clicked when the Shift modifier is pressed, it should select the innermost morph, *regardless* whether another morph has halos or not.
>
> Please fix that.  I would recommend simply reverting back to my method, which is 10X easier to read and understand, but its your call.  I care more about the _functionality_ than the code style, but I do think my effort brought a significant improvement to that method.
>
> Best,
>   Chris
>
> On Thu, May 31, 2018 at 2:09 AM, Marcel Taeumel <[hidden email]> wrote:
> Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/
>
> You changed ALL the formatting, making it close to impossible to make out what you actually changed.
>
> Before:
>
> <image.png>
>
> After:
>
> <image.png>
>
> This is bad. :-(
>
>  You might have fixed a bug, which one? Maybe I can help here.
>
> Best,
> Marcel
>> Am 31.05.2018 08:04:48 schrieb H. Hirzel <[hidden email]>:
>>
>> Hi Chris
>>
>> Are you working on getting the Connectors [1] add-on to work fine
>> with Squeak 5.2a?
>> Recently I loaded Connectors into Squeak6.0a and it loaded fine.
>> However I did not do any tests yet.
>>
>> --Hannes
>>
>> [1] Connectors
>> Connectors is a package by Ned Konz that turns Morphic into a drawing
>> environment for making connected drawings.
>> ...
>> This project has been ported to Squeak 4.x by Chris Muller, and the
>> updated code is available at SqueakSource
>>
>> Current and old versions are available at SqueakMap!
>>
>>
>>
>> On 5/31/18, Marcel Taeumel wrote:
>> > Hey Chris,
>> >
>> > both #layoutChanged and #changed MUST work for all morphs even if the morph
>> > is not in a world or has no owner. Please revert this change and fix
>> > NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>> >
>> > Best,
>> > Marcel
>> > Am 30.05.2018 23:51:51 schrieb [hidden email]
>> > :
>> > Chris Muller uploaded a new version of Morphic to project The Trunk:
>> > http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Morphic-cmm.1443
>> > Author: cmm
>> > Time: 30 May 2018, 4:50:16.716113 pm
>> > UUID: 2f269941-1afa-4615-9242-9e485ecd986e
>> > Ancestors: Morphic-mt.1442
>> >
>> > - When updating a borderStyle:, only signal changes if the receiver is in
>> > the world for them to be seen. This fixes:
>> > NCAAConnectorMorph new
>> > - When halo's are on a background Morph, and a foreground morph covering it
>> > is blue-clicked, transfer the halos to the clicked morph.
>> > - Disable the new #balanceOffsets when Smart Splitters is enabled,
>> > unfortunately they currently don't play nicely together.
>> >
>> > =============== Diff against Morphic-mt.1442 ===============
>> >
>> > Item was changed:
>> > ----- Method: Morph>>borderStyle: (in category 'accessing') -----
>> > borderStyle: aBorderStyle
>> > -
>> > aBorderStyle = self borderStyle ifTrue: [^ self].
>> > -
>> > "If we cannot draw the new border, accept at least its color and width."
>> > ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
>> > ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
>> > ifFalse: [
>> > self borderStyle
>> > width: aBorderStyle width;
>> > baseColor: aBorderStyle baseColor].
>> > -
>> > self borderStyle trackColorFrom: self.
>> > + self isInWorld ifTrue:
>> > + [ self
>> > + layoutChanged;
>> > + changed ]!
>> > -
>> > - self
>> > - layoutChanged;
>> > - changed.!
>> >
>> > Item was changed:
>> > ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
>> > -----
>> > + tryInvokeHalo: aUserInputEvent
>> > + "Invoke halos around the top-most world container at aUserInputEvent's
>> > #position. If it was already halo'd, zero-in on its next inward component
>> > morph at that position. Holding Shift during the click reverses this
>> > traversal order."
>> > + | stack target owners |
>> > + Preferences noviceMode ifTrue: [^self ].
>> > + Morph haloForAll ifFalse: [^self].
>> > + "the stack is the top-most morph to bottom-most."
>> > + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
>> > [ : each | each wantsHaloFromClick ].
>> > + stack ifEmpty: [^self].
>> > + target :=
>> > + aUserInputEvent hand halo
>> > + ifNil: [ stack first ]
>> > + ifNotNil:
>> > + [ : existingHalo |
>> > + stack allButFirst "halo's are always topmost on the stack"
>> > + detect: [ : each | each owner == self ]
>> > + ifFound:
>> > + [ : topMostWhereClicked |
>> > + (existingHalo target withAllOwners includes: topMostWhereClicked)
>> > + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
>> > ]
>> > + ifFalse:
>> > + [ "Transfer halo to new world container."
>> > + aUserInputEvent hand removeHalo.
>> > + aUserInputEvent shiftPressed
>> > + ifTrue: [ stack second ]
>> > + ifFalse: [ topMostWhereClicked ] ] ]
>> > + ifNone: ["Okay, invoke halos on the world." self ] ].
>> > + "With a modifier, we want the innermost, otherwise the outermost."
>> > + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
>> > ].
>> > + "the last owner is expected to be self."
>> > + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
>> > owners at: owners size-1 ifAbsent: [self] ].
>> > + "Now that we have the target, show the halo. Abort event dispatching, too,
>> > to avoid confusion."
>> > + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
>> > + target invokeHaloOrMove: aUserInputEvent.
>> > + "aUserInputEvent has been consumed, don't let it cause any further
>> > side-effects."
>> > + aUserInputEvent ignore!
>> > - tryInvokeHalo: anEvent
>> > -
>> > - | innerMost target |
>> > - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
>> > handle transfer itself."].
>> > - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
>> > - Morph haloForAll ifFalse: [^ self].
>> > -
>> > - innerMost := (self morphsAt: anEvent position unlocked: true) first.
>> > -
>> > - "1) Try to use innermost morph but skip all the ones that do not want to
>> > show a halo along the owner chain."
>> > - target := innerMost.
>> > - [target isNil or: [target wantsHaloFromClick]]
>> > - whileFalse: [target := target owner].
>> > - target ifNil: [^ self].
>> > -
>> > - "2) Without a modifier, which is normal, find the outermost container for
>> > that inner morph."
>> > - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
>> > - | previousTargets |
>> > - previousTargets := OrderedCollection new.
>> > - [target notNil and: [target owner ~~ self]] whileTrue: [
>> > - previousTargets add: target.
>> > - target := target owner].
>> > - target ifNil: [^ self].
>> > - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
>> > - target := previousTargets removeLast].
>> > - target wantsHaloFromClick ifFalse: [^ self]].
>> > -
>> > - "3) Now that we have the target, show the halo. Abort event dispatching,
>> > too, to avoid confusion."
>> > - anEvent hand newMouseFocus: target event: anEvent.
>> > - target invokeHaloOrMove: anEvent.
>> > - anEvent ignore.!
>> >
>> > Item was changed:
>> > ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
>> > -----
>> > mouseUp: anEvent
>> > + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
>> > showTemporaryCursor: nil ].
>> > + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
>> > + traceMorph ifNotNil:
>> > + [ traceMorph delete.
>> > + traceMorph := nil ].
>> > - (self bounds containsPoint: anEvent cursorPoint)
>> > - ifFalse: [anEvent hand showTemporaryCursor: nil].
>> > - self class fastSplitterResize
>> > - ifTrue: [self updateFromEvent: anEvent].
>> > - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
>> > self color: self getOldColor.
>> > + "balanceOffsets currently disrupts Smart Splitter behavior."
>> > + (ProportionalSplitterMorph smartVerticalSplitters or: [
>> > ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
>> > balanceOffsets ]!
>> > -
>> > - self balanceOffsets!
>> >
>> > Item was changed:
>> > ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
>> > step
>> > splitsTopAndBottom
>> > ifTrue: [ self reduceTopBottomImbalance ]
>> > ifFalse:
>> > [ self reduceLeftRightImbalance abs > 1 ifTrue:
>> > [ self splittersLeftDo:
>> > [ : splitter | splitter reduceLeftRightImbalance ].
>> > self splittersRightDo:
>> > + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
>> > - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
>> > - self balanceOffsets!
>> >
>> >
>> >
>>
>
>
>
>
>