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

fix: some forms component can't be disable #7786

Merged

Conversation

Nicoss54
Copy link
Collaborator

@Nicoss54 Nicoss54 commented Dec 23, 2022

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?

if the component is bind to a formControl and the property nzDisable is set to true initially the form element will stay enable. This is not the correct behavior. Form component should be disable

Issue Number: 7726 (release v15)

What is the new behavior?

Form component bind by ngModel and property nzDisable set to true are now disable initially (this was the behavior in version 14)

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@zorro-bot
Copy link

zorro-bot bot commented Dec 23, 2022

This preview will be available after the AzureCI is passed.

@Nicoss54 Nicoss54 mentioned this pull request Dec 23, 2022
@codecov
Copy link

codecov bot commented Dec 23, 2022

Codecov Report

Merging #7786 (1e77042) into master (285655a) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7786      +/-   ##
==========================================
- Coverage   92.08%   92.05%   -0.04%     
==========================================
  Files         502      502              
  Lines       16782    16800      +18     
  Branches     2562     2570       +8     
==========================================
+ Hits        15454    15465      +11     
- Misses       1059     1064       +5     
- Partials      269      271       +2     
Impacted Files Coverage Δ
components/cascader/cascader.component.ts 97.94% <100.00%> (ø)
components/checkbox/checkbox.component.ts 98.68% <100.00%> (+0.03%) ⬆️
components/date-picker/date-picker.component.ts 94.46% <100.00%> (ø)
components/input-number/input-number.component.ts 99.60% <100.00%> (+<0.01%) ⬆️
components/radio/radio.component.ts 97.67% <100.00%> (+0.05%) ⬆️
components/rate/rate.component.ts 96.96% <100.00%> (+0.07%) ⬆️
components/select/select.component.ts 92.60% <100.00%> (+0.31%) ⬆️
components/slider/slider.component.ts 92.36% <100.00%> (+0.08%) ⬆️
components/switch/switch.component.ts 97.56% <100.00%> (+0.06%) ⬆️
components/time-picker/time-picker.component.ts 92.51% <100.00%> (-1.55%) ⬇️
... and 4 more

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

@Nicoss54 Nicoss54 changed the title fix(module:date-picker): nzDatePicker can be always disable fix: some forms can't be disable Dec 27, 2022
@Nicoss54 Nicoss54 changed the title fix: some forms can't be disable fix: some forms component can't be disable Dec 27, 2022
@Nicoss54
Copy link
Collaborator Author

@simplejason can you check this PR when you have a little time. This PR fix some regression due to the bump to angular v15

@simplejason
Copy link
Member

Thanks for your PR :) I will review it ASAP.

@jornare
Copy link

jornare commented Jan 2, 2023

I would assume some updated specs may be appropriate, as this bug was not caught before.

@Nicoss54
Copy link
Collaborator Author

Nicoss54 commented Jan 2, 2023

I would assume some updated specs may be appropriate, as this bug was not caught before.

I am working on it

@simplejason
Copy link
Member

I would assume some updated specs may be appropriate, as this bug was not caught before.

I am working on it

What's the progress of this PR? Can I start to review and merge it?

@Nicoss54
Copy link
Collaborator Author

Nicoss54 commented Jan 3, 2023

I would assume some updated specs may be appropriate, as this bug was not caught before.

I am working on it

What's the progress of this PR? Can I start to review and merge it?

The PR #7786 is finished and can be reviewed. The Pr regarding the missing test is still in progress on my side. I will try to finished it this week.

@jornare
Copy link

jornare commented Jan 3, 2023

It would be great to get this in ASAP as the current release is partially broken.

@Nicoss54
Copy link
Collaborator Author

Nicoss54 commented Jan 3, 2023

It would be great to get this in ASAP as the current release is partially broken.

@simplejason @jornare missing test are added

@Nicoss54 Nicoss54 force-pushed the fix/wrong-disable-behavior-date-picker branch from e0169d1 to 305568b Compare January 3, 2023 16:35
@Nicoss54 Nicoss54 requested review from simplejason and removed request for vthinkxie, wzhudev and wenqi73 January 4, 2023 16:00
@Nicoss54 Nicoss54 marked this pull request as draft January 4, 2023 16:21
@Nicoss54 Nicoss54 marked this pull request as ready for review January 4, 2023 23:31
@Nicoss54
Copy link
Collaborator Author

Nicoss54 commented Jan 4, 2023

@simplejason i think it's goog now :)

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

@jornare
Copy link

jornare commented Jan 5, 2023

Great work guys! Looking forward to getting this in as it is the only thing blocking for my project to go ng15 now :)

@simplejason simplejason merged commit bc673e7 into NG-ZORRO:master Jan 9, 2023
@rqx110
Copy link

rqx110 commented Jan 10, 2023

you are missing nz-radio-group

@Nicoss54 Nicoss54 deleted the fix/wrong-disable-behavior-date-picker branch January 10, 2023 08:08
@Nicoss54
Copy link
Collaborator Author

you are missing nz-radio-group

#7806

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.

4 participants