Skip to content

Resolve some issues with reopening full screen windows - #5280

Merged
Jacalz merged 3 commits into
fyne-io:developfrom
xfaris:develop
Sep 5, 2025
Merged

Jacalz merged 3 commits into
fyne-io:developfrom
xfaris:develop

Conversation

@xfaris

@xfaris xfaris commented Nov 21, 2024 •

Copy link
Copy Markdown
Contributor

Description:

Resolve the issue of incorrect position when opening a full screen window after it is hidden.
Resolve the issue of flickering when the mouse is placed on the taskbar when the full screen window is hidden and opened again.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Resolve the issue of incorrect position when opening a full screen window after it is hidden.
Resolve the issue of flickering when the mouse is placed on the taskbar when the full screen window is hidden and opened again.
@xfaris
xfaris marked this pull request as draft November 21, 2024 09:08
@xfaris
xfaris marked this pull request as ready for review November 21, 2024 09:09
@Jacalz

Jacalz commented Nov 21, 2024

Copy link
Copy Markdown
Member

If the issue is a result of a reach condition then we should solve the race condition instead of sleeping and re-running a function. If we are certain that it is not, we should document clearly why we do it and which issue it relates to.

Do you have an issue number for this problem?

@andydotxyz

Copy link
Copy Markdown
Member

If the issue is a result of a reach condition then we should solve the race condition instead of sleeping and re-running a function. If we are certain that it is not, we should document clearly why we do it and which issue it relates to.

I think they just copied the code from the initial show function.
Perhaps factoring that out into a function called from both places would be clearer?

@coveralls

coveralls commented Nov 21, 2024 •

Copy link
Copy Markdown

Coverage Status

coverage: 62.317% (+0.02%) from 62.302%
when pulling 607264f on xfaris:develop
into 03cd53f on fyne-io:develop.

@Jacalz

Jacalz commented Nov 27, 2024

Copy link
Copy Markdown
Member

I think they just copied the code from the initial show function.
Perhaps factoring that out into a function called from both places would be clearer?

I'm just generally sceptical about creating a new goroutine and then sleeping. How do we know that the arbitrary sleep time is the right one? Why do we even have to spawn a goroutine and then do the operation at a later time? Seems to me like we are either working around a bug or some strange race condition.

Excuse my ramblings though. If this is the only way to solve the problem then sure, a common function with an explanatory comment would be much better.

@andydotxyz

Copy link
Copy Markdown
Member

If this is the only way to solve the problem then sure, a common function with an explanatory comment would be much better.

This sounds like a good conclusion.

In 2.6.0 there may be a better solution as the window display sequence may be affected by other threading changes.

@Jacalz Jacalz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Per discussion above, lets move this out to a separate function

@Jacalz

Jacalz commented Dec 22, 2024

Copy link
Copy Markdown
Member

It looks like this timeout isn't needed with the changes in #5328?

@andydotxyz

Copy link
Copy Markdown
Member

Yes, with that merged @xfaris should rebase on latest develop and the problem goes away.

@andydotxyz

Copy link
Copy Markdown
Member

@xfaris are you able to rebase this and complete the PR? it should be cleaner now that window handling on develop has been tidied up.

@Jacalz

Jacalz commented Sep 3, 2025

Copy link
Copy Markdown
Member

I took the liberty of rebasing this onto latest develop. It would be good with a reproduction example for the issue so we can verify that this fixes the issue. As I have added a commit, I am no longer able to approve and merge this myself.

@Jacalz
Jacalz merged commit 0328dc1 into fyne-io:develop Sep 5, 2025
16 of 19 checks passed
@Jacalz

Jacalz commented Sep 5, 2025

Copy link
Copy Markdown
Member

This is now merged. Sorry for the long wait.

cpustejovsky pushed a commit to cpustejovsky/fyne that referenced this pull request Sep 12, 2025
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