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

feat(material-experimental/mdc-slider): add test harnesses #22648

Merged
merged 1 commit into from May 12, 2021

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented May 9, 2021

Adds test harnesses for the MDC-based slider components.

Adds test harnesses for the MDC-based slider components.
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround merge safe target: minor This PR is targeted for the next minor release labels May 9, 2021
@crisbeto crisbeto requested a review from devversion as a code owner May 9, 2021 09:24
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 9, 2021

/** Harness for interacting with a MDC mat-slider in tests. */
export class MatSliderHarness extends ComponentHarness {
// TODO(wagnermaciel): Implement this in a separate PR
static hostSelector = '.mat-mdc-slider';
Copy link
Member Author

@crisbeto crisbeto May 9, 2021

Choose a reason for hiding this comment

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

I was on the fence on whether to implement separate harnesses for range and non-range sliders since the non-range is the most common case and having one harness means that the consumer would have to go through getStartThumb all the time. I decided against it in order to match what the component's API looks like, but I'm open to changing it. If consumers find it annoying, they could query for MatSliderThumbHarness directly as well.

}

/** Gets the end thumb of the slider. Will throw an error for a non-range slider. */
async getEndThumb(): Promise<MatSliderThumbHarness> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could follow MDC's implementation (link) and use getThumb and getThumbStart similar to their getValue and getValueStart. I'm not saying we should - I actually like the way things are here - but I figured I should leave this suggestion

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's do it like this for now and we can revisit if folks find it difficult to use. I'm not a fan of repeating the same methods in the thumb and the slider.

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label May 11, 2021
@mmalerba mmalerba merged commit 287834a into angular:master May 12, 2021
@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 Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants