-
Notifications
You must be signed in to change notification settings - Fork 135
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 create app step 'valid' which disabled stepper next in production #2566
Conversation
richard-cox
commented
Jun 28, 2018
- In production the statusChanges emits before we watch it for the result
- when we start watching it doesn't emit the now valid form state
- only seen in production when navigating to stepper, not when loading stepper
- fixes CF/Org/Space selector widget with 1 org and 1 space - user can not proceed #2467
- In production the statusChanges emits before we watch it for the result - when we start watching it doesn't emit the now valid form state - only seen in production when navigating to stepper, not when loading stepper
Hey richard-cox! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## v2-master #2566 +/- ##
=============================================
+ Coverage 70.42% 70.44% +0.02%
=============================================
Files 594 594
Lines 25135 25139 +4
Branches 5675 5676 +1
=============================================
+ Hits 17701 17709 +8
+ Misses 7434 7430 -4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- We've got the `valid` pattern wrong in the steppers, the steps are child components and change the state of their parent components - Added this workaround, but we should think about changing this pattern (not sure how though, given that each step controls it's own valid state) - Have also tested other steppers for the same issue and this looks like the only one
@nwmac I've updated to fix the ExpressionChangedAfterItHasBeenCheckedError issues, however needs a re-review |
@richard-cox Feels like a hack to use setTimeout |
…edError" This reverts commit 18a384d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice