-
Notifications
You must be signed in to change notification settings - Fork 199
Add environment variables group #1755
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
Conversation
features/environment-variables.yml
Outdated
| @@ -0,0 +1,7 @@ | |||
| name: CSS environment variables | |||
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.
The concern I have with having this separate from the safe-area-inset feature is that you can't use env() on its own. It was introduced for safe-area-insets, and nothing else at the time.
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 is empirically true, but not at all how it's been presented to web developers. The MDN page introduces it as an abstract thing, the CSS syntax certainly makes it look like one, and the fallback value complicates things further (e.g., should we expect developers to know the history of safe-area-inset to learn if env(invalid, 100px) works?).
That said, maybe this is just an esoteric feature that's only going to have a small number of variables. If this is an evolutionary dead-end (i.e., other spec authors are not considering introducing even more variables), then combining might still be the right choice.
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.
In talking with @mirisuzanne, env() variables come up often in spec discussions, but seldom ends up in the final spec. I merged the environment-variables feature with safe-area-insets, and think the environment-variables group will be useful here. WCO is split into a separate PR.
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.
Yeah, I tend to agree with @captainbrosset here that env() is only a syntax structure for delivering other features. It's like asking when browsers added support for 'css built-in functions' as a concept, instead of asking about support of each individual function. It makes some sense to teach broadly about how function syntax works, but you would almost never ask when that concept gained support, apart from specific features that use it.
features/titlebar-area.yml
Outdated
| @@ -0,0 +1,9 @@ | |||
| name: Titlebar area environment variables | |||
| description: The `titlebar-area-*` CSS environment variables represent the header area of a Progressive Web App that is not hidden by the system's buttons. For example, ` height:\ env(titlebar-area-height)` sets the element's height to the height of the space reserved for the titlebar. | |||
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 we need to mention the phrase "Window Controls Overlay" somewhere in there, for searchability. This feature is only useful if the PWA is a desktop one, only if it is actually installed, only if the manifest defines the window-controls-overlay display-mode, and only if the user actually opted-in to that display mode after having installed the app.
Only then do the titlebare-area-* variables have non-0 values.
We clearly can't say all of this in there, how about this:
| description: The `titlebar-area-*` CSS environment variables represent the header area of a Progressive Web App that is not hidden by the system's buttons. For example, ` height:\ env(titlebar-area-height)` sets the element's height to the height of the space reserved for the titlebar. | |
| description: The `titlebar-area-*` CSS environment variables represent the area that the titlebar of a PWA that uses the `window-controls-overlay` display-mode normally occupies. For example, `height:\ env(titlebar-area-height)` sets the element's height to the height of the space normally reserved for the titlebar. |
That said, how about this different direction:
- We keep
env()andsafe-area-insetas one feature. - We create a second feature called
window-controls-overlay, which includes the newtitlebar-area-*variables + the other WCO-related bits (Navigator.windowControlsOverlay, WindowControlsOverlay, WindowControlsOverlayGeometryChangeEvent, ...)
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.
👍 to a feature for all of windows control overlay—that makes a lot of sense to me. If you want to, you could separate that out into a new PR.
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.
The separate WCO feature is in #1762
features/environment-variables.yml
Outdated
| @@ -0,0 +1,7 @@ | |||
| name: CSS environment variables | |||
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 is empirically true, but not at all how it's been presented to web developers. The MDN page introduces it as an abstract thing, the CSS syntax certainly makes it look like one, and the fallback value complicates things further (e.g., should we expect developers to know the history of safe-area-inset to learn if env(invalid, 100px) works?).
That said, maybe this is just an esoteric feature that's only going to have a small number of variables. If this is an evolutionary dead-end (i.e., other spec authors are not considering introducing even more variables), then combining might still be the right choice.
features/titlebar-area.yml
Outdated
| @@ -0,0 +1,9 @@ | |||
| name: Titlebar area environment variables | |||
| description: The `titlebar-area-*` CSS environment variables represent the header area of a Progressive Web App that is not hidden by the system's buttons. For example, ` height:\ env(titlebar-area-height)` sets the element's height to the height of the space reserved for the titlebar. | |||
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.
👍 to a feature for all of windows control overlay—that makes a lot of sense to me. If you want to, you could separate that out into a new PR.
There's a case to be made to merge
safe-area-insetwithenvironment-variables, but I think there's enough interest to have them separate.I think the "CSS" in "CSS environment variables" is appropriate here- even though other web tech doesn't have environment variables, it's more commonly used in build and backend tools, so I think differentiation is helpful.