docs(manifest): Improvements to orientation page
#35977
Conversation
dipikabh
requested review from
chrisdavidmills
and removed request for
a team
|
Preview URLs (comment last updated: 2024-09-30 03:42:23) |
There was a problem hiding this comment.
Hi @dipikabh! Thanks for your awesome work on this, such an improvement on the existing manifest pages. I love that you are taking this on.
I have some comments for you — mostly not serious.
|
|
||
| - `orientation` | ||
|
|
||
| - : A string with a keyword value, which includes: |
There was a problem hiding this comment.
| - : A string with a keyword value, which includes: | |
| - : A string specifying an `orientation` keyword. Possible values are as follows: |
"with a keyword value" just seemed like strange wording to me.
There was a problem hiding this comment.
Agree, it is reading odd. I wanted to convey that the value can be one of the keywords. This might be better:
- : A string that specifies the default orientation for the web app. The value must be one of the following keywords:
|
|
||
| - `natural` | ||
|
|
||
| - : Displays the web app in the most natural orientation(s) for the device, as determined by the operating system, browser, user settings or the screen itself. |
There was a problem hiding this comment.
| - : Displays the web app in the most natural orientation(s) for the device, as determined by the operating system, browser, user settings or the screen itself. | |
| - : Displays the web app in the most natural orientation for the device, as determined by the screen itself, screen orientation, operating system, browser, and any relevant user settings. |
There was a problem hiding this comment.
It was a bit unclear to me why you were saying "orientation(s)" — it is not as if you can display an app in more than one orientation at once. I think it is fair to assume that we are talking about the orientation of a device at one particular moment, and then list screen orientation as a deciding factor.
There was a problem hiding this comment.
Valid point. I don't know why I wrote "orientation(s). Updating as:
- : Displays the web app in the orientation most natural for the device, as determined by the browser, operating system, user settings, or the screen itself.
Also notice the slight tweak in "n the orientation most natural for the device"
|
|
||
| On devices with a natural portrait orientation: | ||
|
|
||
| - `portrait-primary` is 0 degrees. |
There was a problem hiding this comment.
It is not clear what these degree values mean. Do you mean these are the angles the app will display at for different keyword choices, for portrait/landscape screens? It would be good to specify this clearly.
There was a problem hiding this comment.
This and your other questions on the value descriptions made me look again into app orientation vs screen orientation. Thanks for the nudge!
I've now redone all value descriptions and removed the reference to angles (those are mentioned more with relations to screen orientations (https://www.w3.org/TR/screen-orientation/#the-current-screen-orientation-type-and-angle)). The manifest orientation spec links to all things screen orientation so it was tricky to not use up everything that the screen orientation spec mentions.
I've now also added some more clarifying info in the "Description" section to differentiate between device vs screen vs app orientation.
| - `portrait-secondary` | ||
|
|
||
| - : Displays the web app in the secondary portrait mode. | ||
| This is typically the default `portrait` orientation. |
There was a problem hiding this comment.
Really? It seems odd to me that portrait-secondary would be the default portrait orientation. portrait-primary sounds like it would be the default!
There was a problem hiding this comment.
Likely might have been a copy paste error. Thanks for spotting! Moved this to portrait-primary
|
|
||
| > [!NOTE] | ||
| > The orientation can be changed at runtime via the [Screen Orientation API](/en-US/docs/Web/API/Screen_Orientation_API). | ||
| > While the `orientation` manifest member sets the default orientation of the web app, the orientation of a top-level browsing context can be changed once the web app is running. This can be done through various means, such as using the [Screen Orientation API](/en-US/docs/Web/API/Screen_Orientation_API). |
There was a problem hiding this comment.
Is this note needed? It basically says the same thing as your bullets above. This seems repetitive as-is.
There was a problem hiding this comment.
Yeah, removed it from here. In fact, sticking thes note back to the top of the page where it was in the previous version
|
|
||
| ## Values | ||
| Remember that although it is encouraged, browsers may or may not implement the Screen Orientation API. |
There was a problem hiding this comment.
This wording is a bit odd. Maybe change it to something like "The Screen Orientation API has limited browser support. Check compatibility carefully before using."?
There was a problem hiding this comment.
Also maybe make it a note if you agree with my previous comment about removing the note.
There was a problem hiding this comment.
Added this note:
Note
The Screen Orientation API's lock() method has limited support across browsers.
Check its compatibility if you plan to use this it to dynamically change screen orientation during runtime.
|
|
||
| ### Setting the default orientation for a photo viewing and editing app | ||
|
|
||
| Consider this example of a photo viewing and editing app that works on both mobile phones and tablets. In the app's manifest file, as shown below, the `orientation` is set to `any`, allowing the app to launch in the device's current orientation. The main gallery view of the app adapts to both `portrait` and `landscape` orientations. When a user selects an image for detailed viewing or editing, the app can use the Screen Orientation API to suggest `landscape` orientation for a wider editing canvas on tablets. |
There was a problem hiding this comment.
Thinking about how patchy Screen Orientation API support is, is it wise to recommend using it for this kind of purpose? The Screen Orientation API docs suggests not, and actually link to a guide where this is done using CSS media queries.
There was a problem hiding this comment.
Agree, removed the reference to the API and made the example prose a bit more generic
|
This pull request has merge conflicts that must be resolved before it can be merged. |
dipikabh
force-pushed
the
manifest-orientation
branch
from
5490499
to
54c2cf5
Compare
|
Hi Chris, thanks for the excellent review suggestions and questions. I ended up redoing a lot of content, so it took a bit longer. |
Description
This work is part of improving the web/manifest docs.
Apart from normalizing the page layout to include "Syntax" and "Values", this PR includes the following changes:
Motivation
To ensure all sections have sufficient explanation, all caveats from spec are covered, and the pages follow a similar template
Additional details
Spec links:
Related issues and pull requests
Tracking issue: mdn/mdn#560
Fixes #34927