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

Replace Window::safe_area with Window::insets #4008

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

aecsocket
Copy link

@aecsocket aecsocket commented Nov 25, 2024

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

#3890 added the Window::safe_area API, for getting the distances from the window edge which are safe to draw important content in. This PR replaces window.safe_area() with window.insets(InsetKind::SafeArea):

  • removes safe_area() and adds insets(InsetKind)
  • adds enum InsetKind with only one variant (for now): SafeArea
  • moves some safe_area docs to InsetKind::SafeArea

This opens the door to being able to read the size of different window InsetKinds in the future, such as the inset of window control buttons, or the inset of a specific hardware feature like a camera notch. My use case is to be able to access InsetKind::WindowControls, to read how much space the window's native control buttons take up (if they're not rendered in the title bar).

Tested on Wayland.

Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Hmm, I'm not against the idea of other insets, but I think I'm kinda inclined to have it as separate methods? I.e. adding fn window_controls(&self) -> PhysicalInsets<u32> instead.
That would match what we do elsewhere (e.g. we don't have fn size(&self, kind: SizeKind) and enum SizeKind { Outer, Surface }).

Or do you imagine other inset kinds than InsetKind::WindowControls / can you give a sketch of how you imagine that would work?

I remember it being talked about on Matrix, but I can't remember the details.

@madsmtm
Copy link
Member

madsmtm commented Nov 25, 2024

(That said, thanks for submitting the PR with such a change first, that makes it easier to make API decisions - I'm only lacking the context ;) )

@aecsocket
Copy link
Author

In my eyes, the appeal of using an InsetKind is to not pollute Window with many variants of similar but slightly different functions; I imagine there can be quite a few use cases for InsetKind:

  • SafeArea
  • WindowControls
  • CameraNotch

As functions, all of these would be effectively fn(&self) -> PhysicalInsets<u32>.

But also, I think adding some conceptual separation between insets and windows is useful: for example, Window::camera_notch_size feels off, since it sort of implies that windows have camera notches (which doesn't make sense). Whereas with window.insets(InsetKind::CameraNotch), it feels more like you're acknowledging the fact that the notch is an inset (and may be 0), and it's not as "closely tied" to the window as an associated fn would be.

But at the end of the day, API design can be very subjective. I don't entirely mind which approach is used.

Matrix context: https://matrix.to/#/!DGpLzJTRzBDTwZiogk:matrix.org/$2WKcLxzPkX47I6F9Ngc4qjCzOLWyEIOzO1PMUjDSV7w?via=matrix.org&via=mozilla.org&via=catgirl.cloud

madsmtm: I guess something like window.insets(InsetsKind::SafeArea) is fine?

@madsmtm madsmtm added C - nominated Nominated for discussion in the next meeting S - api Design and usability labels Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - nominated Nominated for discussion in the next meeting S - api Design and usability
Development

Successfully merging this pull request may close these issues.

2 participants