Skip to content

Use the new "retargeting algorighm" in the DOM spec. #8

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 31, 2016
Merged

Use the new "retargeting algorighm" in the DOM spec. #8

merged 1 commit into from
Aug 31, 2016

Conversation

TakayoshiKochi
Copy link
Member

Now the DOM spec version is the canonical link for "retargeting algorighm".

This change is in response for #4 (comment).

@TakayoshiKochi
Copy link
Member Author

@hayatoito PTAL

@TakayoshiKochi TakayoshiKochi mentioned this pull request Aug 29, 2016
Merged
hayatoito
hayatoito reviewed Aug 30, 2016
View reviewed changes
index.html
@@ -384,7 +384,7 @@
<dfn title="pointerLockElement"></dfn>

<p>While the pointer is locked, returns the result of the
<a href="https://w3c.github.io/webcomponents/spec/shadow/#dfn-retargeting-algorithm">retargeting algorithm</a>
<a href="https://dom.spec.whatwg.org/#retarget">retargeting algorithm</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use the similar wording as DOM standard uses?

Let relatedTarget be the result of invoking retargeting event’s relatedTarget against target if event’s relatedTarget is non-null, and null otherwise.

You can use this sentense as a reference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed and updated.

hayatoito
hayatoito reviewed Aug 30, 2016
View reviewed changes
index.html
@@ -410,8 +411,8 @@
<code>document.pointerLockElement</code> returns <code>#host1</code>, and
<code>root1.pointerLockElement</code> returns <code>#canvas1</code>.
The result of
<a href="https://w3c.github.io/webcomponents/spec/shadow/#dfn-retargeting-algorithm">retargeting algorithm</a>
with <code>#root2</code> and <code>#canvas1</code> as input is
<a href="https://dom.spec.whatwg.org/#retarget">retargeting</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of "invoking"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@hayatoito
Copy link
Member

LGTM.

@TakayoshiKochi
Copy link
Member Author

@scheib Could you take a look?

@scheib
Copy link
Contributor

scheib commented Aug 30, 2016

DOM spec has unnatural English grammar,

Example from that spec that make sense:

"Run the retargeting steps with event"

But this doesn't:

"Let relatedTarget be the result of invoking retargeting event’s
relatedTarget against target if event’s relatedTarget is non-null, and null
otherwise"

It would make sense as:

"Let relatedTarget be the result of retargeting event’s relatedTarget
against target if event’s relatedTarget is non-null, and null otherwise"

"Let relatedTarget be the result of invoking retarget on event’s
relatedTarget against target if event’s relatedTarget is non-null, and null
otherwise"

"Let relatedTarget be the result of invoking retarget against target with
event’s relatedTarget if it is non-null, and null otherwise"

So, for Pointer Lock I would suggest:

"returns the result of retargeting with the context object..."

On Mon, Aug 29, 2016 at 7:14 PM, Takayoshi Kochi notifications@github.com
wrote:

@scheib https://github.com/scheib Could you take a look?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAXnm2eVtSm-FEPmMi4v9hOIXIPycFAzks5qk5H5gaJpZM4JvW11
.

@TakayoshiKochi
Use the definition of "retargeting" in the DOM spec.
c93718f 
Now the DOM spec version is the canonical link for "retargeting",
refer to it and adjust wording around the term.
@TakayoshiKochi
Copy link
Member Author

I picked your first example (~ the result of retargeting ~ against ~) rather than "returns the result of retargeting with the context object" (your suggestion at the bottom) as it fits more with the current sentence, IMO. Does this look okay?

@scheib
Copy link
Contributor

scheib commented Aug 30, 2016

LGTM, will wait for Hayato

On Mon, Aug 29, 2016 at 8:52 PM, Takayoshi Kochi notifications@github.com
wrote:

I picked your first example (~ the result of retargeting ~ against ~)
rather than "returns the result of retargeting with the context object"
(your suggestion at the bottom) as it fits more with the current sentence,
IMO. Does this look okay?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAXnm6llrA9_1nJ12cvjdu4-tTkSEOX8ks5qk6jkgaJpZM4JvW11
.

@TakayoshiKochi
Copy link
Member Author

@hayatoito does this look good to you?

@hayatoito
Copy link
Member

Yeah, LGTM.

TakayoshiKochi added a commit to TakayoshiKochi/dom that referenced this pull request Aug 30, 2016
@TakayoshiKochi
Drop "invoking" from "invoking retargeting"
b9da205 
This was pointed out during pointerLock spec change review:
w3c/pointerlock#8
@TakayoshiKochi TakayoshiKochi mentioned this pull request Aug 30, 2016
Closed
@TakayoshiKochi
Copy link
Member Author

FYI I made a PR for DOM: whatwg/dom#318 for the same wording.

@scheib scheib merged commit 03f09d3 into w3c:gh-pages Aug 31, 2016
@TakayoshiKochi TakayoshiKochi deleted the retarget branch August 31, 2016 00:31
annevk pushed a commit to whatwg/dom that referenced this pull request Aug 31, 2016
@TakayoshiKochi @annevk
Editorial: drop "invoking" from "invoking retargeting"
de60f84 
This was pointed out during Pointer Lock change review:
w3c/pointerlock#8.

PR: #318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TakayoshiKochi @hayatoito @scheib