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

Announcer should be revisited. #670

Open
plantec opened this issue Dec 11, 2024 · 5 comments
Open

Announcer should be revisited. #670

plantec opened this issue Dec 11, 2024 · 5 comments
Labels

Comments

@plantec
Copy link
Collaborator

plantec commented Dec 11, 2024

Announcer could be used in BlText instead of BlTextAnnouncer

BlTextAnnouncer allow one to prevent Announcement classes individually but this feature is not used and could be ported at Announcer level if we really need it.

plantec added a commit that referenced this issue Dec 12, 2024
… a fix for #670)

Introduce BlElement>>focusHolder to allow an element to specify the element that should be focused when a focus is requested.
@plantec
Copy link
Collaborator Author

plantec commented Dec 13, 2024

BlTextAnnouncer is not removed for now because the method #postCopy remains. I wonder why Announcer doesn't have the same

@tinchodias
Copy link
Collaborator

Maybe we suggest the change in Pharo. But did you check if Bloc actually copies the announcer?

@Ducasse Ducasse changed the title Announcer could be used in BlText instead of BlTextAnnouncer Announcer should be revisited. Jan 9, 2025
@Ducasse
Copy link
Contributor

Ducasse commented Jan 9, 2025

There are some points to clarify and comment.

The storedAnnouncements is only used for test.

@Ducasse
Copy link
Contributor

Ducasse commented Jan 9, 2025

in addition

announce: anAnnouncement

	^ self isSuspended
		  ifFalse: [
			  | announcement |
			  announcement := anAnnouncement asAnnouncement.

			  "If the user required to prevent the current announcement, we stop here."
			  (self preventedAnnouncements handlesAnnouncement: announcement) ifTrue: [ ^ self ].

			  registry ifNotNil: [ registry deliver: announcement ].
			  announcement ]
		  ifTrue: [ storedAnnouncements ifNotNil: [ storedAnnouncements add: anAnnouncement ] ]

storedAnnouncements can contain non announcement

@Ducasse
Copy link
Contributor

Ducasse commented Jan 9, 2025

We should remove the asAnnouncement on Symbol and on Announcement class

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

No branches or pull requests

3 participants