Add Window controls overlay #1762
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
feature definition
…indow-controls-overlay
captainbrosset
approved these changes
Sep 11, 2024
Contributor
|
In case other reviewers want to learn more about WCO before reviewing: |
ddbeck
requested changes
Sep 11, 2024
| @@ -0,0 +1,18 @@ | |||
| name: Window Controls Overlay | |||
Collaborator
There was a problem hiding this comment.
I hate it when we have to capitalize features, but this one is so weirdly named I don't think we have a reasonable alternative.
| - css.properties.custom-property.env.titlebar-area-height | ||
| - css.properties.custom-property.env.titlebar-area-width | ||
| - css.properties.custom-property.env.titlebar-area-x | ||
| - css.properties.custom-property.env.titlebar-area-y |
Collaborator
There was a problem hiding this comment.
Suggested change
| - css.properties.custom-property.env.titlebar-area-y | |
| - css.properties.custom-property.env.titlebar-area-y | |
| - html.manifest.display_override.window-controls-overlay |
features/window-controls-overlay.yml
Outdated
| @@ -0,0 +1,18 @@ | |||
| name: Window Controls Overlay | |||
| description: The Window Controls Overlay API displays content in the title bar area of a progressive web app that is installed on desktop and has `display_override:\ window-controls-overlay` in the manifest. | |||
Collaborator
There was a problem hiding this comment.
This contains almost everything we need, but does require some clean ups:
- If we're mentioning the main entry point, we should lead with it like we do for HTML elements, CSS properties, and some APIs.
- Every other example of
display_overrideI could find had an array of strings. I don't think you'd ever literally typedisplay_override: window-controls-overlay, but I don't know this feature well and could be corrected. - "on desktop" sounds clipped, so I went with "desktop device" (see also)
- Due to tech writer esoterica, replacing avoiding "displays" as a verb.
Suggested change
| description: The Window Controls Overlay API displays content in the title bar area of a progressive web app that is installed on desktop and has `display_override:\ window-controls-overlay` in the manifest. | |
| description: The `display_override: ["window-controls-overlay"]` web application manifest member shows content in the title bar area of a progressive web app that is installed on a desktop device. |
Contributor
There was a problem hiding this comment.
display_override: ["window-controls-overlay"] is the correct syntax, sorry I missed it in my previous review. This manifest member provides a list of display-mode fallbacks. When using the display-mode member, you don't get to specify a sequence of fallbacks, only the mode you prefer, and you then let the browser choose how to fallback in case a mode isn't possible.
…indow-controls-overlay
ddbeck
approved these changes
Sep 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split off from #1755