Skip to content
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

Submission/microsoft/pointer events #324

Merged
merged 1 commit into from Jun 3, 2014
Merged

Submission/microsoft/pointer events #324

merged 1 commit into from Jun 3, 2014

Conversation

jacobrossi
Copy link
Contributor

Contains an initial set of Pointer Event test case submissions from Microsoft and also a bug fix to an existing test case. Additional submissions will follow.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/305

This is an external review system which you may optionally use for the code review of your pull request.

@ghost ghost assigned mbrubeck Sep 9, 2013
</h4>
<br />
<div id="target0">
Prese and hold a mouse button,or use touch or pen to contact this element. Move around inside the element while maintaining contact/button down.
Copy link
Contributor

Choose a reason for hiding this comment

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

Some typos here: s/Prese/Press/ and s/,or/, or/

@cathychan
Copy link

Some general comments.

  1. It would be helpful to annotate the tests with test assertion numbers [TA] and/or annotate the test assertion page with test file names.
  2. Many tests include an instruction to run the test again with a different pointer type, but the instructions are not consistent:
    * Refresh the page to run the tests again with a different pointer type.
    * Refresh the page to run the tests again with a different pointer type?
    * Would you like to <a href="#">run the tests again </a> with a different pointer type?
    They should all use the first variant. (Note that <a href="#"></a> would not enable the user to rerun the test.)

[TA] http://www.w3.org/wiki/PointerEvents/TestAssertions

@cathychan
Copy link

Also, can we have more consistent file names please? Now we have

  • pointercancel.html
  • pointerout.html
  • pointerup.html

but

  • pointerevent_pointerEnter.html
  • pointerevent_pointerOver.html
  • pointerevent_pointerLeave_xxx.html

etc.

test(function () {
assert_equals(event.tiltX, 0, event.type + ".tiltX is 0 for mouse");
assert_equals(event.tiltY, 0, event.type + ".tiltY is 0 for mouse");
assert_equals(event.pointerId, 1, event.type + ".pointerId is 1 for mouse");

Choose a reason for hiding this comment

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

This is no longer in the spec and should be removed.

@cathychan
Copy link

A few more general comments.

  1. All tests need a done(); statement at test completion. Otherwise, the page would show a "Harness timed out" even if the test is completed. (Currently, pointercancel.html is the only test that has this statement.)
  2. Some of the #complete-notice styles are missing "background: #afa;".
  3. Test file names should be consistent. Right now, we have pointercancel.html, pointerout.html and pointerup.html, but pointerevent_pointerEnter.html, pointerevent_pointerOver.html, etc.

if (eventTested == false) {
detected_pointertypes[event.pointerType] = true;

test(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

fix indentation of this block

@sideshowbarker
Copy link
Contributor

@ArtemAntonets, @jacobrossi what's this PR still waiting on before we can merge it?

@ArtemAntonets
Copy link

All comments processed except 3 that I've commented here. And also 1 from here: https://critic.hoppipolla.co.uk/showcomment?chain=3710

Simon asks to add "_manual" to the pages that require manual interaction that will lead to renaming all the files except just 2 that are automatic. This will make review process to be very difficult. In case this change is reasonable, I would suggest to create according issue and make a separate PR with only name change.

@AFBarstow
Copy link
Contributor

@ArtemAntonets - please note the file naming convention for manual tests is to use a dash ("-") and not an underscore ("_"). Thus, f.ex. "pointerleave-manual.html"

@jacobrossi jacobrossi merged commit 73533e2 into web-platform-tests:master Jun 3, 2014
jacobrossi added a commit that referenced this pull request Jun 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants