Skip to content

feat(material-experimental/mdc-slider): implement some basic unit tests #22072

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

Merged
merged 7 commits into from
Mar 9, 2021

Conversation

wagnermaciel
Copy link
Contributor

  • implement unit tests for the standard slider, standard range slider, and for the slider ripple states
  • add mdc-slider theme to all-theme
  • make MatSliderVisualThumb ripple refs public so their states can be checked & tested
  • make MatSlider child component and html element getters default thumbPosition to the end thumb

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 1, 2021
@wagnermaciel wagnermaciel marked this pull request as ready for review March 1, 2021 21:10
@wagnermaciel wagnermaciel requested a review from devversion as a code owner March 1, 2021 21:10
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM, with one more comment

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM with a few nits.

@wagnermaciel wagnermaciel requested a review from a team as a code owner March 4, 2021 16:34
@wagnermaciel wagnermaciel force-pushed the mdc-slider-unit-tests branch 3 times, most recently from 30f540b to 7b22164 Compare March 5, 2021 14:35
* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* make MatSliderVisualThumb ripple refs public so their states can be checked & tested
* make MatSlider child component and html element getters default thumbPosition to the end thumb
* use #waitForAsync instead of #fixture.whenStable
@wagnermaciel wagnermaciel force-pushed the mdc-slider-unit-tests branch from 7b22164 to d2d3ca5 Compare March 5, 2021 14:57
* remove default value from MatSlider getters
* combine #beforeEach's in ripple state tests
* resolve injection error that only throws on ci
* make ripple refs private and check DOM directly in unit tests
@wagnermaciel wagnermaciel force-pushed the mdc-slider-unit-tests branch from c3e4461 to cd25296 Compare March 5, 2021 19:44
* Disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch.
  The problem is easily reproduced if you undo this change and test it out on a mobile device.
@josephperrott josephperrott removed the request for review from a team March 8, 2021 18:02
@wagnermaciel
Copy link
Contributor Author

I noticed a bug while trying to fix the unit tests on for ios safari. Here are some screenshots of the fix:

Before

Screen Shot 2021-03-08 at 9 57 12 AM

After

Screen Shot 2021-03-08 at 9 50 08 AM

It's pretty difficult to see, but there was a ripple getting launched by default on touch. You can see it if you look at the bottom left corner closely. The ripple was never centered so it would appear wherever the slider thumb was touched. To fix this I just disabled the mat ripple (ripples can still be launched manually when the mat ripple is disabled)

// In favor of creating events that work for most of the browsers, the event is created
// as a basic UI Event. The necessary details for the event will be set manually.
const event = document.createEvent('UIEvent');
const touchDetails = {pageX, pageY};
const touchDetails = {pageX, pageY, clientX, clientY};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mdc-slider checks the clientX and clientY of the targetTouches[0] when calculating the new value of the slider.

https://github.com/material-components/material-components-web/blob/master/packages/mdc-slider/foundation.ts#L372-L374

* use touch events instead of pointer events when testing on ios.
  pointerdown, pointerup, and pointermove are not supported on ios.
@wagnermaciel wagnermaciel force-pushed the mdc-slider-unit-tests branch from b3cca71 to 17d55b2 Compare March 8, 2021 21:33
@wagnermaciel wagnermaciel requested a review from mmalerba March 8, 2021 22:03
@wagnermaciel wagnermaciel added the target: feature This PR is targeted for a feature branch (outside of main and semver branches) label Mar 8, 2021
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@wagnermaciel wagnermaciel merged commit cbc9ca0 into angular:mdc-slider Mar 9, 2021
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 11, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 11, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 12, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 23, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Apr 1, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Apr 2, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Apr 8, 2021
…ts (angular#22072)

* feat(material-experimental/mdc-slider): implement some basic unit tests

* implement unit tests for the standard slider, standard range slider, and for the slider ripple states
* add mdc-slider theme to all-theme
* use #waitForAsync to wait for foundation to finish initializing & layout
* use forwardRef to avoid injection errors that only throw on ci
* disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch
  the problem is easily reproduced if you undo this change and test it out on a mobile device.
* note: we use touch events instead of pointer events when testing on ios because pointerdown,
  pointerup, and pointermove are not supported
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement target: feature This PR is targeted for a feature branch (outside of main and semver branches)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants