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

fix(module: date-picker): fix year-picker and month-picker style erro… #2136

Merged
merged 2 commits into from Sep 19, 2018

Conversation

Projects
None yet
2 participants
@hungtcs
Contributor

hungtcs commented Sep 14, 2018

…r within compacted input group

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: 2084

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@wendzhue wendzhue requested a review from wilsoncook Sep 14, 2018

@@ -146,7 +146,7 @@ describe('NzMonthPickerComponent', () => {
it('should support nzClassName', () => {
const className = fixtureInstance.nzClassName = 'my-test-class';
fixture.detectChanges();
const picker = debugElement.query(By.css('.ant-calendar-picker')).nativeElement as HTMLElement;
const picker = debugElement.queryAll(By.css('.ant-calendar-picker'))[1].nativeElement as HTMLElement;

This comment has been minimized.

@vthinkxie

vthinkxie Sep 17, 2018

Member

it seems it has nothing todo with the pr with this code.

This comment has been minimized.

@hungtcs

hungtcs Sep 17, 2018

Contributor

@vthinkxie Two elements in the picker has css class ant-calendar-picker, but the test target is the second element. I found the same code in date-picker.component.spec.ts

This comment has been minimized.

@vthinkxie

vthinkxie Sep 17, 2018

Member

@hungtcs great, can you add some test for the bug you fixed? thanks.

This comment has been minimized.

@hungtcs

hungtcs Sep 17, 2018

Contributor

@vthinkxie It's hard to test styles via javascript, see this stackblitz

This comment has been minimized.

@vthinkxie

vthinkxie Sep 17, 2018

Member

@hungtcs the test is must for ci, you can use https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle to test the style in javascript.

This comment has been minimized.

@hungtcs

hungtcs Sep 17, 2018

Contributor

@vthinkxie Thank you, I will add the test case later.

This comment has been minimized.

@vthinkxie

This comment has been minimized.

@hungtcs

hungtcs Sep 18, 2018

Contributor

@vthinkxie Test case is completed.

hungtcs added some commits Sep 14, 2018

@vthinkxie vthinkxie merged commit 049212f into NG-ZORRO:master Sep 19, 2018

3 of 4 checks passed

codecov/project 96.01% (-0.01%) compared to 21f0703
Details
codecov/patch Coverage not affected when comparing 21f0703...9f232dc
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment