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(module:date-picker): add nzShowWeekNumber property #7621

Merged

Conversation

UncleDave
Copy link
Contributor

@UncleDave UncleDave commented Aug 31, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] 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?

Week numbers are only shown in the week picker component.

Issue Number: Fixes #3764

What is the new behavior?

[nzShowWeekNumber] can control whether week numbers are shown in the date picker. Week numbers are still always shown in the week picker.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@zorro-bot
Copy link

zorro-bot bot commented Aug 31, 2022

This preview will be available after the AzureCI is passed.

@UncleDave
Copy link
Contributor Author

@wenqi73 I know you guys have a huge stack of open PRs, but do you have any sort of estimate for when this PR might be reviewed?

@wenqi73
Copy link
Member

wenqi73 commented Sep 7, 2022

@UncleDave CI failed, you should make it run successfully first.

@UncleDave
Copy link
Contributor Author

@wenqi73 I'm not familiar with your CI pipeline, but I don't see how my changes could have caused this failure, nor can I reproduce it locally, do you have any clues about it?

✖ Compiling with Angular sources in Ivy partial compilation mode.
components/core/testing/component-bed.ts:38:5 - error TS2322: Type 'TestBed' is not assignable to type 'TestBedStatic'.
  Type 'TestBed' provides no match for the signature 'new (...args: any[]): TestBed'.

38     bed,
       ~~~

  components/core/testing/component-bed.ts:13:3
    13   bed: TestBedStatic;
         ~~~
    The expected type comes from property 'bed' which is declared here on type 'ComponentBed<T>'

[07:33:54] 'library:build-zorro' errored after 44 s
[07:33:54] Error: Process failed with code 1
    at ChildProcess.<anonymous> (/home/vsts/work/1/s/scripts/gulp/util/task-helpers.ts:34:25)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:552:15)
    at maybeClose (node:internal/child_process:1093:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)
    at Process.callbackTrampoline (node:internal/async_hooks:130:17)

@wenqi73
Copy link
Member

wenqi73 commented Sep 7, 2022

@UncleDave Run npm run test and npm run build can reproduce this.

@UncleDave
Copy link
Contributor Author

@wenqi73 I ran those before my commit, they both succeed for me.

@wenqi73
Copy link
Member

wenqi73 commented Sep 7, 2022

@UncleDave Maybe you should merge the latest code of master and re-install dependencies?

@codecov
Copy link

codecov bot commented Sep 7, 2022

Codecov Report

Merging #7621 (03242d5) into master (fb49c04) will increase coverage by 1.55%.
The diff coverage is 100.00%.

❗ Current head 03242d5 differs from pull request most recent head 2de6ef0. Consider uploading reports for the commit 2de6ef0 to get more accurate results

@@            Coverage Diff             @@
##           master    #7621      +/-   ##
==========================================
+ Coverage   90.43%   91.99%   +1.55%     
==========================================
  Files         502      502              
  Lines       16752    16738      -14     
  Branches     2629     2629              
==========================================
+ Hits        15150    15398     +248     
+ Misses       1314     1072     -242     
+ Partials      288      268      -20     
Impacted Files Coverage Δ
components/core/testing/component-bed.ts 90.90% <ø> (ø)
...mponents/date-picker/date-range-popup.component.ts 95.16% <ø> (ø)
components/date-picker/inner-popup.component.ts 86.36% <ø> (ø)
...ponents/pagination/pagination-default.component.ts 97.61% <ø> (ø)
...ponents/pagination/pagination-options.component.ts 92.59% <ø> (ø)
...mponents/pagination/pagination-simple.component.ts 92.15% <ø> (ø)
components/date-picker/date-picker.component.ts 93.69% <100.00%> (ø)
components/date-picker/lib/abstract-table.ts 93.87% <100.00%> (+0.12%) ⬆️
components/date-picker/lib/date-table.component.ts 97.18% <100.00%> (ø)
components/table/src/styled/ellipsis.directive.ts 50.00% <0.00%> (-50.00%) ⬇️
... and 57 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@UncleDave UncleDave force-pushed the feat/date-picker-week-numbers branch 2 times, most recently from d9ffe94 to 577c336 Compare September 8, 2022 08:46
@UncleDave
Copy link
Contributor Author

@wenqi73 Is there no package-lock.json file? It looks like my build is now failing due to dependency type mismatches. You can also see that every other PR is now failing with the same issue that I originally had on this branch. Does this project have a solution for locking dependencies? It's very hard to make a change and have confidence it will build on the CI server.

package.json Outdated Show resolved Hide resolved
Copy link
Member

@wenqi73 wenqi73 left a comment

Choose a reason for hiding this comment

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

I left some comments.

Copy link
Member

@wenqi73 wenqi73 left a comment

Choose a reason for hiding this comment

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

LGTM

@UncleDave
Copy link
Contributor Author

@wenqi73 Is there anything else you need from me here? The code cov changes are weird since it says the coverage of ellipsis.directive.ts has changed, but I don't really see how. I can try to write a test for that if needed?

Copy link
Member

@simplejason simplejason left a comment

Choose a reason for hiding this comment

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

LGTM

@simplejason simplejason merged commit 2cb80fc into NG-ZORRO:master Sep 24, 2022
@UncleDave UncleDave deleted the feat/date-picker-week-numbers branch September 24, 2022 15:17
chenc041 pushed a commit to chenc041/ng-zorro-antd that referenced this pull request Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to show week numbers on all datepickers
3 participants