Skip to content

proposal to bubble up events happening on shapes inside a composite - #156

Open
Pynci wants to merge 1 commit into
pharo-graphics:Pharo14from
Pynci:Pharo14
Open

proposal to bubble up events happening on shapes inside a composite#156
Pynci wants to merge 1 commit into
pharo-graphics:Pharo14from
Pynci:Pharo14

Conversation

@Pynci

@Pynci Pynci commented May 25, 2026

Copy link
Copy Markdown

No description provided.

@Ducasse

Ducasse commented May 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for this contribution.
@akevalion can you have a look?

@Ducasse

Ducasse commented May 27, 2026

Copy link
Copy Markdown
Contributor

Why do you need this type check?
Could not we have parent isnil do something?

@Ducasse

Ducasse commented May 27, 2026

Copy link
Copy Markdown
Contributor

Do you have tests showing the expected behavior?

@Pynci

Pynci commented May 27, 2026

Copy link
Copy Markdown
Author

Not yet, I apologize

@akevalion

Copy link
Copy Markdown
Contributor

Hi @Pynci let us know so I can review your changes with new tests


| theEventToSend |

(self parent isKindOf: RSComposite) ifTrue: [ self parent announce: anEventOrAnEventClassOrABlock ].

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is good to avoid isKindOf: when possible. In this case, something like self parent bubbleEvent: anEventOrAnEventClassOrABlock can be a replacement.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(However, I didn't analyze if my suggestion results in a proper implementation of bubbling, I am only talking of how to replace the isKindOf:.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! My pull request was in the first place a way to say "I found it often useful to have events bubbled up in the composite, does it make sense in the first place?". If you think it makes sense, I can then try to find a nice solution (avoiding type checks) :D

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

Successfully merging this pull request may close these issues.

4 participants