- From: Yoav Weiss <notifications@github.com>
- Date: Tue, 27 Jul 2021 23:27:40 -0700
- To: w3c/editing <editing@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/editing/pull/317/review/716606029@github.com>
@yoavweiss commented on this pull request.
Getting closer... A few more comments
> @@ -107,6 +106,17 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
attribute EventHandler ongeometrychange;
};
</pre>
+ <p>
+ The {{VirtualKeyboard}} object has an associated:
+ </p>
+ <dfn>boundingRect</dfn>
+ <p>
+ A {{DOMRect}}, initially has zero values.
+ </p>
+ <dfn>overlaysContent</dfn>
+ <p>
+ A boolean, initially `false`. When this attribute is set to `true`, a user agent MUST NOT resize its layout viewport or <a href="https://wicg.github.io/visual-viewport/#dom-visualviewport">visual viewport</a>.
IntersectionObserver uses the "document's viewport", pointing to https://drafts.csswg.org/css2/#viewport
That seems more precise than "layout viewport".
> @@ -116,7 +126,11 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
The method must follow these steps:
</p>
<ol>
- <li>If the browsing context's <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-window">active window</a> does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
+ <li>
+ Let |window| be [=this=]'s <a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a>. If |window| is not a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a>, then abort these steps.
I *think* you can replace the abort with an assertion that |window| is a [=Window=] (because this is only exposed on Window)
Nit: You can replace the explicit links (here and elsewhere) with spec references. e.g. "Let |window| be [=this=]'s [=relevant global object=]. Assert that |window| is a {{Window}} object."
> </dt>
<dd>
<p>
- The attribute reports the intersection of the VK with the layout viewport in client coordinates.
+ The attribute reports the intersection of the VK with the layout viewport in client coordinates. It is set when {{VirtualKeyboard/ongeometrychange}} event is fired. To create a {{VirtualKeyboard/boundingRect}} follow these steps:
+ <ol>
+ <li>
+ Let `osk` be the on-screen keyboard rectangle in CSS pixels that is provided by the OS when the VK visibility changes.
All references to variables (osk, lv, bounds, etc) should be of the form "|var|" rather than "`var`". That enables various features such as variable highlighting, and makes it clearer that they are in fact vaariables
> </dt>
<dd>
<p>
- The attribute reports the intersection of the VK with the layout viewport in client coordinates.
+ The attribute reports the intersection of the VK with the layout viewport in client coordinates. It is set when {{VirtualKeyboard/ongeometrychange}} event is fired. To create a {{VirtualKeyboard/boundingRect}} follow these steps:
No need to mention when it is set here. It'd be sufficient to define an algorithm here (e.g. "set a virtual keyboard bounding rect"), and then call it before the event is fired.
> @@ -128,7 +142,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
Call the system API to show the VK.
</li>
<li>
- When the VK is shown by the system, fire {{VirtualKeyboard/ongeometrychange()}} event.
+ When the VK is shown by the system, <a href="https://html.spec.whatwg.org/#in-parallel">in parallel</a> the OS reports the bounding rectangle of the keyboard. Update the {{VirtualKeyboard/boundingRect}} which is the intersection of the keyboard rectangle reported by the OS and the layout viewport. Fire {{VirtualKeyboard/ongeometrychange()}} event to JS.
You can move the "in parallel" bit up, into something like:
"""
<li>[=In parallel=], follow these steps:
* Wait for the virtual keyboard to be shown by the system.
* Call "set the virtual keyboard bounding rect" with the keyboard's OS reported bounding rectangle and the [=document=]'s [=viewport=].
* [=Fire an event=] named "geometrychange" at [=this=].
> @@ -140,17 +154,21 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
The method must follow these steps:
</p>
<ol>
- <li>If the browsing context is not focused, abort these steps.
+ <li>
+ Let |window| be [=this=]'s <a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a>. If |window| is not a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a>, then abort these steps.
Same comment as above on abort and links
> @@ -116,7 +126,11 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
The method must follow these steps:
</p>
<ol>
- <li>If the browsing context's <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-window">active window</a> does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
+ <li>
+ Let |window| be [=this=]'s <a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a>. If |window| is not a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a>, then abort these steps.
+ </li>
+ <li>
+ If |window| does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
Same nit on replacing links with references
> Call the system API to hide the VK.
- </li>
- <li>
- When the VK is hidden by the system, {{VirtualKeyboard/ongeometrychange()}} event is fired that contains the intersection of the keyboard geometry and layout viewport in client coordinates.
- </li>
+ </li>
+ <li>
+ When the VK is hidden by the system, <a href="https://html.spec.whatwg.org/#in-parallel">in parallel</a> the OS reports the bounding rectangle of the keyboard (which has all 0 values). Create a {{DOMRect}} with 0 values and return it when {{VirtualKeyboard/boundingRect}} is queried. Fire {{VirtualKeyboard/ongeometrychange()}} event to JS.
Same comment as above on revamping to multiple steps
> </dt>
<dd>
<p>
- The attribute reports the intersection of the VK with the layout viewport in client coordinates.
+ The attribute reports the intersection of the VK with the layout viewport in client coordinates. It is set when {{VirtualKeyboard/ongeometrychange}} event is fired. To create a {{VirtualKeyboard/boundingRect}} follow these steps:
+ <ol>
+ <li>
+ Let `osk` be the on-screen keyboard rectangle in CSS pixels that is provided by the OS when the VK visibility changes.
+ </li>
+ <li>
+ Let `lv` be the layout viewport's bounds in CSS pixels.
+ </li>
+ <li>
+ Let `bounds` be a {{DOMRect}} object.
+ </li>
+ <li>
+ Set `bounds` to be the intersection of `lv` and `osk` in CSS pixels.
It'd be better to do something like https://www.w3.org/TR/intersection-observer/#calculate-intersection-rect-algo here. You can't really reuse it as is, because |osk| is not a DOM element...
> </dt>
<dd>
<p>
- The attribute reports the intersection of the VK with the layout viewport in client coordinates.
+ The attribute reports the intersection of the VK with the layout viewport in client coordinates. It is set when {{VirtualKeyboard/ongeometrychange}} event is fired. To create a {{VirtualKeyboard/boundingRect}} follow these steps:
+ <ol>
+ <li>
+ Let `osk` be the on-screen keyboard rectangle in CSS pixels that is provided by the OS when the VK visibility changes.
Also, |osk| and |lv| should be input parameters, and the caller should provide them
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/editing/pull/317#pullrequestreview-716606029
Received on Wednesday, 28 July 2021 06:27:53 UTC