fix(module:time-picker): placeholder not update with i18n changes #6069

DepickereSven · 2020-11-18T14:03:38Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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?

When you update the i18n.setLocale() after a few seconds the placeholder of the time-picker component will not update.

Reproduce https://stackblitz.com/edit/angular-qyneqn?file=src/app/app.component.ts

What is the new behavior?

Whenever you update i18n.setLocale() the placeholder will also update.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

zorro-bot · 2020-11-18T14:03:41Z

This preview will be available after the AzureCI is passed.

codecov · 2020-11-18T14:09:43Z

Codecov Report

Merging #6069 (0d50558) into master (860dfed) will decrease coverage by 0.00%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master    #6069      +/-   ##
==========================================
- Coverage   89.82%   89.81%   -0.01%     
==========================================
  Files         469      477       +8     
  Lines       14948    15206     +258     
  Branches     2273     2302      +29     
==========================================
+ Hits        13427    13658     +231     
- Misses        935      952      +17     
- Partials      586      596      +10

Impacted Files	Coverage Δ
components/back-top/back-top.component.ts	`89.09% <ø> (ø)`
components/layout/layout.component.ts	`100.00% <ø> (ø)`
components/message/message-container.component.ts	`91.66% <ø> (ø)`
components/page-header/page-header.component.ts	`85.00% <ø> (ø)`
...ponents/time-picker/time-picker-panel.component.ts	`89.02% <ø> (ø)`
components/tooltip/base.ts	`91.03% <ø> (ø)`
components/upload/upload-list.component.ts	`95.86% <ø> (ø)`
components/result/result.component.ts	`92.30% <50.00%> (-0.80%)`	⬇️
components/card/card.component.ts	`93.33% <100.00%> (ø)`
components/checkbox/checkbox-group.component.ts	`100.00% <100.00%> (ø)`
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 860dfed...0d50558. Read the comment docs.

DepickereSven · 2020-12-14T12:59:28Z

@wenqi73 is it possible to review this PR?
It's an annoying bug that the language isn't correct when you reload the page.

wenqi73

Thanks for your contribution, add a unit test example would be better.

wenqi73 · 2020-12-15T02:18:02Z

components/time-picker/time-picker.component.ts

+    this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(
+      map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder),
+      startWith(this.i18n.getLocaleData(`TimePicker.lang.placeholder`))
+    );


Suggested change

this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(

map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder),

startWith(this.i18n.getLocaleData(`TimePicker.lang.placeholder`))

);

this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(

map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder)

);

Already has the initial value, no need to use startWith.

# Conflicts: # components/time-picker/time-picker.component.ts

DepickereSven · 2020-12-15T10:24:43Z

@wenqi73 I added a test case for the change I made.
But I had also a merge conflict to solve.
I resolved the merge conflict with a merge from master and now I have a lot of file changes.
But they are formatting changes, do I need to create a new PR or is it okay like this?

wenqi73 · 2020-12-15T11:15:34Z

components/time-picker/time-picker.component.spec.ts

+  ngOnInit(): void {
+    this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder));
+  }


Suggested change

ngOnInit(): void {

this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder));

}

wenqi73 · 2020-12-15T11:15:45Z

components/time-picker/time-picker.component.spec.ts

  open = false;
  openChange = jasmine.createSpy('open change');
  autoFocus = false;
  date: Date | string = new Date();
  disabled = false;
  use12Hours = false;
  nzSuffixIcon?: string;
+  i18nPlaceHolder$: Observable<string | undefined> = of(undefined);


Suggested change

i18nPlaceHolder$: Observable<string | undefined> = of(undefined);

wenqi73 · 2020-12-15T11:16:22Z

components/time-picker/time-picker.component.spec.ts

@@ -166,20 +188,28 @@ describe('time-picker', () => {
      (ngModelChange)="onChange($event)"
      [(nzOpen)]="open"
      (nzOpenChange)="openChange($event)"
+      [nzPlaceHolder]="i18nPlaceHolder$ | async"


Suggested change

[nzPlaceHolder]="i18nPlaceHolder$ | async"

wenqi73 · 2020-12-15T11:18:22Z

components/time-picker/time-picker.component.spec.ts

+        let placeHolderValue: string | undefined;
+        testComponent.i18nPlaceHolder$.subscribe(v => (placeHolderValue = v));
+
+        expect(placeHolderValue).toBe('请选择时间');


placeholder = timeElement.nativeElement.querySelector('input').placeholder;

wenqi73 · 2020-12-15T11:21:12Z

@DepickereSven The formatting change is OK, no need to change this.

wenqi73

Thanks @DepickereSven !

DepickereSven · 2020-12-15T13:03:14Z

@wenqi73 You're welcome and thank you for reviewing!

Depickere Sven and others added 9 commits September 15, 2020 19:34

fix(module:select): arrow icon can be used when not using single-select

43c80d9

docs(module:select): update docs according to the fix

be8af3d

fix(module:select): fix get statement

53d1f0d

Merge remote-tracking branch 'upstream/master'

86b03ee

Merge remote-tracking branch 'upstream/master'

8795b77

Merge branch 'master' of https://github.com/DepickereSven/ng-zorro-antd

3ecbf6e

Merge branch 'master' of https://github.com/NG-ZORRO/ng-zorro-antd

84eccf0

fix(module:timepicker): placeholder not update with i18n changes

7d97f6b

fix(module:timepicker): fix types

f53ee69

DepickereSven requested a review from wenqi73 as a code owner November 18, 2020 14:03

pr-triage bot added the PR: unreviewed label Nov 18, 2020

DepickereSven changed the title ~~Dep s/time placeholder i18n~~ fix(module:timepicker): placeholder not update with i18n changes Nov 18, 2020

wenqi73 requested changes Dec 15, 2020

View reviewed changes

pr-triage bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Dec 15, 2020

Depickere Sven added 2 commits December 15, 2020 09:19

fix(module:time-picker): rm startWith

fd8c552

test(module:time-picker): added testcase

b57b928

pr-triage bot added PR: unreviewed and removed PR: reviewed-changes-requested labels Dec 15, 2020

Depickere Sven added 2 commits December 15, 2020 11:00

Merge remote-tracking branch 'upstream/master'

d5ea817

Merge branch 'master' into depS/time-placeholder-i18n

682b566

# Conflicts: # components/time-picker/time-picker.component.ts

DepickereSven requested review from cipchk, CK110, hsuanxyz, Ricbet, vthinkxie and wzhudev as code owners December 15, 2020 10:03

DepickereSven requested a review from wenqi73 December 15, 2020 10:24

wenqi73 reviewed Dec 15, 2020

View reviewed changes

test(module:time-picker): update testcase

0d50558

DepickereSven requested a review from wenqi73 December 15, 2020 12:19

wenqi73 approved these changes Dec 15, 2020

View reviewed changes

pr-triage bot added PR: partially-approved and removed PR: unreviewed labels Dec 15, 2020

wenqi73 changed the title ~~fix(module:timepicker): placeholder not update with i18n changes~~ fix(module:time-picker): placeholder not update with i18n changes Dec 15, 2020

wenqi73 merged commit f34840b into NG-ZORRO:master Dec 15, 2020

pr-triage bot added PR: merged and removed PR: partially-approved labels Dec 15, 2020

DepickereSven deleted the depS/time-placeholder-i18n branch December 15, 2020 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(module:time-picker): placeholder not update with i18n changes #6069

fix(module:time-picker): placeholder not update with i18n changes #6069

DepickereSven commented Nov 18, 2020 •

edited

Loading

zorro-bot bot commented Nov 18, 2020

codecov bot commented Nov 18, 2020 •

edited

Loading

DepickereSven commented Dec 14, 2020 •

edited

Loading

wenqi73 left a comment

wenqi73 Dec 15, 2020

DepickereSven commented Dec 15, 2020

wenqi73 Dec 15, 2020

wenqi73 Dec 15, 2020

wenqi73 Dec 15, 2020

wenqi73 Dec 15, 2020

wenqi73 commented Dec 15, 2020

wenqi73 left a comment

DepickereSven commented Dec 15, 2020

	ngOnInit(): void {
	this.i18nPlaceHolder$ = this.i18n.localeChange.pipe(map((nzLocale: NzI18nInterface) => nzLocale.TimePicker.placeholder));
	}

fix(module:time-picker): placeholder not update with i18n changes #6069

fix(module:time-picker): placeholder not update with i18n changes #6069

Conversation

DepickereSven commented Nov 18, 2020 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

zorro-bot bot commented Nov 18, 2020

codecov bot commented Nov 18, 2020 • edited Loading

Codecov Report

DepickereSven commented Dec 14, 2020 • edited Loading

wenqi73 left a comment

Choose a reason for hiding this comment

wenqi73 Dec 15, 2020

Choose a reason for hiding this comment

DepickereSven commented Dec 15, 2020

wenqi73 Dec 15, 2020

Choose a reason for hiding this comment

wenqi73 Dec 15, 2020

Choose a reason for hiding this comment

wenqi73 Dec 15, 2020

Choose a reason for hiding this comment

wenqi73 Dec 15, 2020

Choose a reason for hiding this comment

wenqi73 commented Dec 15, 2020

wenqi73 left a comment

Choose a reason for hiding this comment

DepickereSven commented Dec 15, 2020

DepickereSven commented Nov 18, 2020 •

edited

Loading

codecov bot commented Nov 18, 2020 •

edited

Loading

DepickereSven commented Dec 14, 2020 •

edited

Loading