Skip to content

feat(module:input): support one time password (OTP)#8715

Merged
Laffery merged 49 commits into
NG-ZORRO:masterfrom
ParsaArvanehPA:feature/input-otp
Nov 25, 2024
Merged

feat(module:input): support one time password (OTP)#8715
Laffery merged 49 commits into
NG-ZORRO:masterfrom
ParsaArvanehPA:feature/input-otp

Conversation

@ParsaArvanehPA

@ParsaArvanehPA ParsaArvanehPA commented Aug 26, 2024

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • 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?

Implementation of the OTP component of the Ant Desgin

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • [✔] No

Other information

Since we are currently on the fourth version of Ant, and the varient section is one of the changes made to the input component in the fifth version, this part is not currently implemented.

@zorro-bot

zorro-bot Bot commented Aug 26, 2024

Copy link
Copy Markdown

This preview will be available after the AzureCI is passed.

@codecov

codecov Bot commented Aug 26, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.39130% with 7 lines in your changes missing coverage. Please review.

Project coverage is 91.89%. Comparing base (0202a19) to head (2db319c).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
components/input/input-otp.component.ts 92.39% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8715      +/-   ##
==========================================
- Coverage   91.89%   91.89%   -0.01%     
==========================================
  Files         546      547       +1     
  Lines       19291    19383      +92     
  Branches     2850     2866      +16     
==========================================
+ Hits        17728    17812      +84     
- Misses       1251     1255       +4     
- Partials      312      316       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@HyperLife1119 HyperLife1119 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@ParsaArvanehPA

Copy link
Copy Markdown
Contributor Author

Hi @Laffery
Could you please take a look at this pr?

@Laffery

Laffery commented Nov 19, 2024

Copy link
Copy Markdown
Collaborator

Hi @Laffery Could you please take a look at this pr?

OK 👌

Comment thread components/input/input-otp.spec.ts Outdated
Comment thread components/input/doc/index.zh-CN.md Outdated
Comment thread components/input/doc/index.en-US.md Outdated
expect(callback).toHaveBeenCalledWith('test value');
});

it('should register onTouched callback', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These methods are implementation of ControlValueAccessor interface, which is well tested by angular.
I think we needn't to write test cases for them, WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right—logically, these parts shouldn't need to be tested again. However, for some reason, Karma can't recognize that they are part of Angular and expects us to test them. By removing these two tests, we would lose some code coverage. What’s your take on this?

@Laffery Laffery changed the title feat(module:input): otp feat(module:input): support one time password (OTP) Nov 19, 2024
@Laffery

Laffery commented Nov 21, 2024

Copy link
Copy Markdown
Collaborator

Hi, @ParsaArvanehPA I had reviewed and left several comments, please take a look

@ParsaArvanehPA

Copy link
Copy Markdown
Contributor Author

Hi, @ParsaArvanehPA I had reviewed and left several comments, please take a look

Hi @Laffery
Thank tou for checking my PR ❤
Your requested changes are made.

@Laffery Laffery left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

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.

3 participants