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

Branch: RedoingClosePresenter, Deleted everything so SpClosedWindowListPresenter can be independant #17676

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

AlexisCnockaert
Copy link
Collaborator

@AlexisCnockaert AlexisCnockaert commented Jan 23, 2025

now SystemWindow has nothing linked with the presenter

Copy link

request-info bot commented Jan 23, 2025

This issue has either a default title or empty body. We would appreciate it if you could provide more information. Note: I am not a very intelligent bot, I can only react to new comments. Please add a comment for me if you update the body or title.

@Ducasse
Copy link
Member

Ducasse commented Jan 23, 2025

But Alexis you still refer to Sp* so we have a dependency from Morphic to Presenter.
Can you check if people are around to know if we have announcement when a window is open or closed.

@AlexisCnockaert
Copy link
Collaborator Author

sorry yes there is an announcement I forgot

@jecisc
Copy link
Member

jecisc commented Jan 23, 2025

We do have announcements yes.

Some browser were missing it but I fixed some cases when I implemented my POC.

Here is the code I used:

self currentWorld announcer
		when: WindowClosed
		do: [ :ann | cache at: DateAndTime now put: ann window ]
		for: self

@AlexisCnockaert
Copy link
Collaborator Author

The problem is it will add a new window on the list everytime, for example if I want to close for real my hidden window, a WindowClosed announcement will be announced too, adding again the window to the list.
Wouldn't it be better to create a special announcement like WindowHidden?

@jecisc jecisc added Status: Need more work The issue is nearly ready. Waiting some last bits. and removed more-information-needed labels Jan 23, 2025
@jecisc
Copy link
Member

jecisc commented Jan 23, 2025

What you can maybe do is to prevent the announcement for the second time.

self currentWorld announcer prevent: WindowClosed during: [ window close ]

@Ducasse
Copy link
Member

Ducasse commented Jan 25, 2025

I wonder if the memory leak I have in the my image does not come from the uncloseable logic so we should really pay attention and be picky.

@Ducasse Ducasse merged commit 2a15025 into pharo-project:Pharo13 Jan 30, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Need more work The issue is nearly ready. Waiting some last bits.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants