Form/action refactoring and other updates#2666
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2666 +/- ##
==========================================
+ Coverage 65.81% 66.15% +0.34%
==========================================
Files 1167 1199 +32
Lines 32528 33303 +775
Branches 5929 6128 +199
==========================================
+ Hits 21409 22033 +624
- Misses 11005 11138 +133
- Partials 114 132 +18
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Since Javascript doesn't really have a concept of package-private, this linting rule should help accomplish something close
Doing this on every render was causing a React error to be logged.
dpwatrous
force-pushed
the
dpwatrous/parameter-dependencies
branch
2 times, most recently
from
bcb0ed0 to
19dd662
Compare
- Properly display form validation errors - Replaced "Parameter types" with Parameter classes - Form dependencies - Dynamic properties - Move update logic into layout components - Support action data loading - New parameters and form controls
dpwatrous
force-pushed
the
dpwatrous/parameter-dependencies
branch
from
19dd662 to
9b62d70
Compare
dpwatrous
requested review from
NickKouds,
cRui861,
gingi,
paterasMSFT,
seth-barshay,
skapur12,
wiboris and
zfengms
as code owners
gingi
left a comment
gingi
left a comment
There was a problem hiding this comment.
Some opportunities for refactoring in the future, but this is great!
| } | ||
|
|
||
| private _validate(): ValidationStatus { | ||
| if (this.value != null && Array.isArray(this.value)) { |
There was a problem hiding this comment.
Seems that if value is not null nor an array, it should raise an error, no?
| if (errorCount === 1 && lastErrorMsg) { | ||
| errorMsg = lastErrorMsg; | ||
| } else { | ||
| errorMsg = `${errorCount} ${ |
There was a problem hiding this comment.
Might want to generalize this a bit for localization (because "X errors found" might not be grammatically correct in other languages): errorCount === 1 ? `1 error found` : `${errorCount} errors found` .
| warningMsg = lastWarningMsg; | ||
| } else { | ||
| warningMsg = `${warningCount} ${ | ||
| warningCount === 1 ? "warning" : "warnings" |
| * Creates a standalone parameter with a backing form. Useful for using | ||
| * form controls outside the context of an entire form | ||
| */ | ||
| export function createParam< |
There was a problem hiding this comment.
Thinking about this more: Why not just put use the constructor (e.g., put the logic in AbstractParameter's constructor)?
2 participants
No description provided.