Skip to content

Conversation

@g1shin
Copy link

@g1shin g1shin commented Aug 31, 2017

No description provided.

@g1shin g1shin requested review from jelbourn, kara and mmalerba August 31, 2017 23:14
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 31, 2017
import {Key} from 'selenium-webdriver';
import {screenshot} from '../screenshot';

fdescribe('stepper', () => {
Copy link
Member

Choose a reason for hiding this comment

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

fdescribe

@@ -0,0 +1,23 @@
<button (click)="isLinear=!isLinear" id="toggle-linear">Enable linear</button>
Copy link
Member

Choose a reason for hiding this comment

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

Rather than creating a new e2e test page, the new preference is to run e2e tests over an example from the material-examples/ package. Check out the card tests as an example of doing this.

Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

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

LGTM other than J's comments

describe('basic behavior', () => {
it('should change steps correctly when stepper button is clicked', async () => {
let previousButton = element.all(by.buttonText('Back'));
let nextButton = element.all(by.buttonText('Next'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: const

@kara kara removed their assignment Sep 1, 2017
@g1shin g1shin requested a review from amcdnl as a code owner September 1, 2017 21:13
@g1shin
Copy link
Author

g1shin commented Sep 1, 2017

Comments have been addressed. Ready for review again 👍

<md-step [stepControl]="firstFormGroup">
<form [formGroup]="firstFormGroup">
<ng-template mdStepLabel>Step one</ng-template>
Content of step one
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 make the example a little more "real"? In general, users will get a better idea of how the component works when it more closely represents what they're be doing with it.

(these are the examples that show up on the docs site)

import {FormBuilder, FormGroup, Validators} from '@angular/forms';

/**
* @title Stepper overview
Copy link
Member

Choose a reason for hiding this comment

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

Now that you have this example, you can add

<!-- example(stepper-overview) -->

To stepper.md (under the first sentence summary) and it will embed the example.

@g1shin
Copy link
Author

g1shin commented Sep 1, 2017

Modified stepper example for the docs site 😃

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Sep 5, 2017
@tinayuangao
Copy link
Contributor

@g1shin please rebase

@g1shin
Copy link
Author

g1shin commented Sep 6, 2017

@tinayuangao rebase done 👍

@tinayuangao tinayuangao merged commit bef6271 into angular:master Sep 6, 2017
@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 Sep 7, 2019
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants