Skip to content

eframe: add set_minimized and set_maximized#2292

Merged
emilk merged 9 commits into
emilk:masterfrom
SunDoge:feat-window-actions
Feb 4, 2023
Merged

eframe: add set_minimized and set_maximized#2292
emilk merged 9 commits into
emilk:masterfrom
SunDoge:feat-window-actions

Conversation

@SunDoge
Copy link
Copy Markdown

@SunDoge SunDoge commented Nov 12, 2022

This PR adds some window actions to enable the possibility to create client-side decorations in egui applications.

window_actions

Copy link
Copy Markdown
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

This is really cool, thanks for working on this!

Comment thread examples/custom_window_frame/src/main.rs Outdated
Comment thread examples/custom_window_frame/src/main.rs Outdated
Comment thread examples/custom_window_frame/src/main.rs
@SunDoge
Copy link
Copy Markdown
Author

SunDoge commented Nov 14, 2022

The cranky step should pass now.

@SunDoge SunDoge requested a review from emilk November 15, 2022 02:07
Copy link
Copy Markdown
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

On my Mac, this PR behaves very badly. The titlebar is back and I cannot move the window 😅

Screen Shot 2022-11-16 at 11 28 05

@SunDoge
Copy link
Copy Markdown
Author

SunDoge commented Nov 16, 2022

Is this problem caused by this PR?

@emilk
Copy link
Copy Markdown
Owner

emilk commented Nov 16, 2022

Yeah, I looked a bit closer. Apearently asking window.is_maximized() is what is causing this. 🤦

And there is no winit event for maximize state changing.

@yunfengwangluo
Copy link
Copy Markdown

When learning EGUI, I found that when I click the maximize button, the window in Windows will ignore the position of the task bar and directly enter the full screen state In ubuntu, the taskbar can be explicitly displayed normally

Under normal circumstances, clicking Maximize should only maximize the window, and should not enter the full screen state!

@SunDoge
Copy link
Copy Markdown
Author

SunDoge commented Nov 26, 2022

Hi @emilk, could you please check if this can be fixed by maintaining the window maximized state in the app?

@SunDoge
Copy link
Copy Markdown
Author

SunDoge commented Nov 26, 2022

Hi @yunfengwangluo, could you show the command you run? I haven't witnessed this behavior.

@emilk
Copy link
Copy Markdown
Owner

emilk commented Nov 27, 2022

It seems winit is quite buggy on Mac. Minimize work, but maximize makes the app hang on the next call to window.fullscreen() (i.e. asking if it is fullscreen).

@emilk
Copy link
Copy Markdown
Owner

emilk commented Jan 23, 2023

So the main winit problem was fixed by rust-windowing/winit#2636, but we need to wait for a new winit release, but that's coming pretty soon:

@emilk
Copy link
Copy Markdown
Owner

emilk commented Feb 4, 2023

It now works well on my mac (with the winit 0.28 update).

Update the min_window_size and initial_window_size in examples/custom_window_frame/src/main.rs to avoid this:

image

@emilk
Copy link
Copy Markdown
Owner

emilk commented Feb 4, 2023

Another nice thing to implement would be double-clicking the title-bar to maximize.

@emilk emilk changed the title feat: add actions for window controls eframe: add set_minimized and set_maximized Feb 4, 2023
@emilk emilk merged commit f0718a6 into emilk:master Feb 4, 2023
lictex pushed a commit to lictex/egui that referenced this pull request Feb 5, 2023
* add actions for window controls

* add maximized to WindowInfo
update button text
fix clippy

* add overlap icon when maximized

* remove argument `app`

* remove WindowInfo { maximized }

* Update minimum window size

* Double-click titlebar to toggle maximized state

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
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.

3 participants