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
[css-env-1] Define safe area inset variables #2629 #2807
Conversation
cc: @tabatkins @grorg |
css-env-1/Overview.bs
Outdated
</pre> | ||
|
||
The safe area insets are four [=environment variables=] that define the top, right, bottom and | ||
left insets from the edge of the document. Page content that is in this safe area may be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'document' seems wrong here - shouldn't it be something like 'viewport'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I've been assuming the safe area is the bit defined by the insets, so it I think it would be 'content that is outside this safe area' or 'content that is inside these insets'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - fixed.
css-env-1/Overview.bs
Outdated
Safe area inset variables {#safe-area-insets} | ||
------------------------------------------------------------------ | ||
|
||
<pre class=propdef> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think defining these using propdef
entries doesn't seem right. The only aspects of the propdef tables that actually seem to apply here are "Name", "Value", and maybe "Media". Furthermore, using propdef entries will put them in property indexes, which will be very confusing.
I think in the absence of a better mechanism this should probably be something more like a hand-written table, and if a better mechanism is needed it can be developed later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
css-env-1/Overview.bs
Outdated
|
||
The safe area insets are four [=environment variables=] that define the top, right, bottom and | ||
left insets from the edge of the viewport. Page content that is inside these insets may be | ||
obscured by the shape of the display. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this description could be clearer about what these insets are for to readers not already familiar with the concept, and should probably also have stronger implementation conformance criteria. I'd suggest rewording to something like the following:
The safe area insets are four [=environment variables=] that define a rectangle by its top, right, bottom, and left insets from the edge of the viewport. For rectangular displays, these must all be zero, but for nonrectangular displays they must form a rectangle, chosen by the user agent, such that all content inside the rectangle is visible, and such that reducing any of the insets would cause some content inside of the rectangle to be invisible due to the nonrectangular nature of the display. This allows authors to limit the layout of essential content to the space inside of the safe area rectangle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Thanks. This looks good to me. Since you kept the "Media" entry in the table, I think the one thing that's still remaining is defining what happens when the medium in question doesn't match. That's beyond the definition of the safe area inset variables. I suspect a decent answer would be that the variable isn't present. But it should probably be defined; I filed #2819 to follow up on that. |
Thanks! |
@rebeccahughes do you work at Google? |
@astearns Yes I do |
Define CSS safe area inset variables as part of #2629