Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feedback from Yann #1

Open
Ducasse opened this issue May 17, 2024 · 3 comments
Open

feedback from Yann #1

Ducasse opened this issue May 17, 2024 · 3 comments

Comments

@Ducasse
Copy link
Member

Ducasse commented May 17, 2024

Salut,

j'ai lu rapidement à partir de la page 14 jusqu'à 33. Voici mes retours:

page 17

On peut écrire le backElement de manière plus simple:

MGCardElement >> initialize
super initialize.
backElement := self class cardbackForm asElement.
self size: self cardExtent.
self layout: BlLinearLayout new alignCenter.
self background: self backgroundPaint.
self geometry: (BlRoundedRectangleGeometry cornerRadius: 12).
self card: (MGCard new symbol: $a).

On peut aussi avoir la taille de l'element backElement avec:

backElement constraints horizontal resizer size @ (backElement constraints vertical resizer size).

On peut utiliser la taille des constraints pour ne plus à gérer les tailles des éléments dans le code de #cardExtent mais c'est peut-être compliqué pour un exemple.

page 18

Je ne sais si j'aurais utilisé un BlLinearLayout pour faire le centrage. J'aurais priviligié un BlFrameLayout + des constraints sur les éléments back et front. Mais je ne suis vraiment pas sur de ça.


page 20

On peut aussi utiliser la visibilité pour switch entre le front et le back.
Arguments pour visibility:

  • on peut avoir des enfants supplémentaires qui peuvent servir à décorer la carte par exemple.
  • déclenche moins d'events.
    Arguments pour remove/add child:
  • on peut facilement avoir plus de 2 états (extensible).
  • plus facile à comprendre du point de vue du développeur ?

page 25

MGGameElement >> initialize
super initialize.
self background: Color veryLightGray.
self layout: (BlGridLayout horizontal cellSpacing: 20).
self
constraintsDo: [ :aLayoutConstrants |
aLayoutConstraints horizontal matchParent.
aLayoutConstraints vertical matchParent ]

erreur au niveau de :aLayoutConstrants -> :aLayoutConstraints

En utilisant fitContent, la taille de MGGameElement correspond quoi qu'il advienne à la taille des cards à l'intérieur.
Il n'y a plus besoin de gérer manuellement la taille du space je pense.

MGGameElement >> initialize
super initialize.
self background: Color veryLightGray.
self layout: (BlGridLayout horizontal cellSpacing: 20).
self
constraintsDo: [ :aLayoutConstraints |
aLayoutConstraints horizontal fitContent.
aLayoutConstraints vertical fitContent ]

page 26

Sans fit content:

MGGameElement class >> openWithNumber
| aGameElement space |
aGameElement := MGGameElement new
memoryGame: MGGame withNumbers;
yourself.
space := BlSpace new.
space root addChild: aGameElement.
space root whenLayoutedDoOnce: [ space extent: 420 @ 420 ].
space show

Avec fit content:

MGGameElement class >> openWithNumber
| aGameElement space |
aGameElement := MGGameElement new
memoryGame: MGGame withNumbers;
yourself.
space := BlSpace new.
space root addChild: aGameElement.
space root whenLayoutedDoOnce: [ space extent: aGameElement size ].
space show

page 38

Il y a un figure ??

Je relirai une seconde fois la semaine prochaine je pense. Pour voir si je trouve d'autres choses.

BC,

Yann

@Nyan11
Copy link
Contributor

Nyan11 commented May 20, 2024

Voir: #2 , #3 et #4.

@Nyan11
Copy link
Contributor

Nyan11 commented May 20, 2024

page 20

On peut aussi utiliser la visibilité pour switch entre le front et le back.

Arguments pour visibility:

  • on peut avoir des enfants supplémentaires qui peuvent servir à décorer la carte par exemple.
  • déclenche moins d'events.

Arguments pour remove/add child:

  • on peut facilement avoir plus de 2 états (extensible).
  • plus facile à comprendre du point de vue du développeur ?

Je n'ai pas fait de PR pour la visibility.
Ca implique beaucoup de changement dans le document.
Et je n'ai pas testé pour voir si ça marche bien coté Pharo

@Ducasse
Copy link
Member Author

Ducasse commented May 20, 2024

Ok on pourra expliquer cela dans le livre sur Bloc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants