Skip to content

feat(module:form): support form label wrap #7892

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

Merged
merged 1 commit into from
Jul 16, 2023

Conversation

TheBeard30
Copy link
Contributor

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?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@zorro-bot
Copy link

zorro-bot bot commented Mar 31, 2023

This preview will be available after the AzureCI is passed.

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #7892 (dc375e2) into master (169181c) will decrease coverage by 0.01%.
The diff coverage is 77.77%.

❗ Current head dc375e2 differs from pull request most recent head ae04d6b. Consider uploading reports for the commit ae04d6b to get more accurate results

@@            Coverage Diff             @@
##           master    #7892      +/-   ##
==========================================
- Coverage   91.80%   91.79%   -0.01%     
==========================================
  Files         509      509              
  Lines       17452    17461       +9     
  Branches     2684     2685       +1     
==========================================
+ Hits        16022    16029       +7     
- Misses       1136     1138       +2     
  Partials      294      294              
Impacted Files Coverage Δ
components/form/form-label.component.ts 79.54% <75.00%> (-1.02%) ⬇️
components/form/form.directive.ts 96.42% <100.00%> (+0.13%) ⬆️

@@ -81,6 +85,17 @@ export class NzFormLabelComponent implements OnDestroy {
}
private _tooltipIcon: NzFormTooltipIcon | 'default' = 'default';

@Input()
set nzLabelWrap(value: boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use @InputBoolean() instead of getter?

<nz-form-label nzLabelWrap></nz-form-label>

Copy link
Member

Choose a reason for hiding this comment

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

Also you need to rebase this with the master branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay👌🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use @InputBoolean() instead of getter?

<nz-form-label nzLabelWrap></nz-form-label>

If the nzLabelWrap of the NzFormLabelComponent has no value, we
need to get the nzLabelWrap of the Form Directive, so i think we need to use getter

Copy link
Member

Choose a reason for hiding this comment

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

@InputBoolean() will automatically set this property to be true, check other components that use this decorator like select

@TheBeard30 TheBeard30 force-pushed the form-label-wrap branch 2 times, most recently from b845863 to 3baa48e Compare April 3, 2023 13:57
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 37391de into NG-ZORRO:master Jul 16, 2023
@TheBeard30 TheBeard30 deleted the form-label-wrap branch October 16, 2024 07:10
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.

2 participants