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

Add Window::get_fullscreen() -> bool #579

Closed
rukai opened this issue Jun 27, 2018 · 4 comments · Fixed by #838
Closed

Add Window::get_fullscreen() -> bool #579

rukai opened this issue Jun 27, 2018 · 4 comments · Fixed by #838
Labels
C - needs discussion Direction must be ironed out S - api Design and usability

Comments

@rukai
Copy link
Contributor

rukai commented Jun 27, 2018

set_fullscreen takes an Option<WindowId>.
However we dont need to make get_fullscreen() return an Option<WindowId> because we can use get_current_monitor() -> WindowId instead.

The window manager (at least on X11) can set the fullscreen state so just remembering the state in winit will not be enough.

@francesca64 francesca64 added S - api Design and usability C - needs discussion Direction must be ironed out labels Jun 27, 2018
@francesca64
Copy link
Member

The way I'd want to address this is similar to #208, since:

  • Maximization events are necessary and it would make sense for the fullscreen API to follow the same pattern.
  • There's also a fullscreen button on macOS.
  • I tend to favor events over getters, since people presumably usually only want to know about changes, and only as often as they happen.

So I'd go with WindowEvent::Fullscreen(Option<MonitorId>). However, I'm not opposed to having the getter as well, though I think it really should return Option<MonitorId>, for consistency with set_fullscreen. That also makes it easier to do dumb stuff with combinators.

@rukai
Copy link
Contributor Author

rukai commented Jun 27, 2018

Thanks, that all makes sense.

@francesca64
Copy link
Member

Are there any WMs you recommend for testing this?

@rukai
Copy link
Contributor Author

rukai commented Jul 19, 2018

Not sure if your asking me directly? I only use i3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out S - api Design and usability
Development

Successfully merging a pull request may close this issue.

2 participants