Conversation
LANDAISB
left a comment
LANDAISB
left a comment
There was a problem hiding this comment.
Need to create Unit Tests ! At least for all commands
| ^ property | ||
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } |
There was a problem hiding this comment.
Need to choose a prototol name
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } | ||
| PyramidBackgroundBlocPlugin class >> gaussianShadowWidth [ |
There was a problem hiding this comment.
Not a good name, prefix with shadow, maybe shadowWidthGaussian
| inputValue: [ PyramidShadowCommand current lastGaussianShadow ]; | ||
| inputValidation: [ :value | value class = BlGaussianShadowEffect ]; | ||
| yourself); | ||
| yourself. |
There was a problem hiding this comment.
Remove the last yourself (unused)
| propertiesManager addProperty: self class borderRadialOuterRadius. | ||
|
|
||
| "Shadow" | ||
| PyramidShadowCommand resetShadowCommand. |
There was a problem hiding this comment.
Why need reset ? Why use a singleton (current) ?
| childToMoveCollection size = 1 | ||
| ifFalse: [ ^ self ]. | ||
| navigationSelectionPanel := navigationPlugin navigation | ||
| selectionPanel. |
There was a problem hiding this comment.
no need to carriage return here
| BlSimpleShadowEffect color: Color black offset: 2 @ 2 ] | ||
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } |
| aBlElement effect: anArgument | ||
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } |
| current := nil | ||
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } |
| { #category : #'as yet unclassified' } | ||
| PyramidShadowCommand >> getValueFor: aBlElement [ | ||
|
|
||
| ^ aBlElement effect |
There was a problem hiding this comment.
There are several types of effects in BlElement : overlay, blur, shadow, etc. How do you handle a BlElement with another effect ?
| aBlElement effect class = BlSimpleShadowEffect ifTrue: [ | ||
| lastSimpleShadow := aBlElement effect ]. | ||
| aBlElement effect class = BlGaussianShadowEffect ifTrue: [ | ||
| lastGaussianShadow := aBlElement effect ] |
There was a problem hiding this comment.
same problem here: different kind of effects
| PyramidShadowOffsetCommand >> getValueFor: aBlElement [ | ||
|
|
||
| ^ aBlElement effect offset | ||
| ^ (aBlElement effect isKindOf: BlShadowEffect) ifTrue: [aBlElement effect offset] ifFalse: [ ^ self ] |
There was a problem hiding this comment.
Why return self fo other effects ? Maybe better to return nil or 0@0 ?
| PyramidShadowColorCommand >> getValueFor: aBlElement [ | ||
| ^ aBlElement effect color | ||
|
|
||
| (aBlElement effect isKindOf: BlShadowEffect) |
There was a problem hiding this comment.
Global on several methods: better to use guard clause => (a = b) ifTrue:[^self].
| ^ PyramidShadowColorCommand new | ||
| ] | ||
|
|
||
| { #category : #'as yet unclassified' } |
There was a problem hiding this comment.
Need protocol name: (fix it on TPyramidCommandTest)
2 participants
update devStage