-
Notifications
You must be signed in to change notification settings - Fork 388
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
New Dynamic Form Feature(s) - Custom Formatting and Validation, ControlsTestWebPart updates #1672
Conversation
…ression validation and custom formatting renderer.
Thanks @t0mgerman for the great addition! But this is definitely a HUGE improvement for the Dynamic Form! |
… use of 'any' Amendment to CustomFormatting class type annotations
Hi @AJIXuMuK - I've acted on all of those points and brought the code up to date with |
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.
Thanks @t0mgerman for this iteration.
It looks solid to me.
I left a few comments that are more "nit"/code style ones, not functionality-wise.
I will delay the merge though before we release a new version. To have more "beta" time for this change.
Thanks again!
private convertCustomFormatExpressionNodes = (node: ICustomFormattingExpressionNode | string | number | boolean): ASTNode => { | ||
if (typeof node !== "object") { | ||
switch (typeof node) { | ||
case "string": |
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.
you can combine both case
in one
stack[parenDepth].push(token); | ||
} else if (token.value === ")") { | ||
// When Right parenthesis is found, items are popped from stack to output until left parenthesis is found | ||
while (stack[parenDepth].length > 0 && stack[parenDepth][stack[parenDepth].length - 1].value !== "(") { |
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.
stack[parentDepth].length > 0
=> stack[parentDepth].length
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.
and everywhere below
@t0mgerman - could you please resolve the conflicts and I will merge the PR? thanks! |
Hi @AJIXuMuK sorry about not getting back sooner, I'll look at getting them resolved today |
Hi @AJIXuMuK - resolved conflicts as of this post 👍 Needed to make a few manual changes to mesh my work together with GuidoZam's file uploading solution (recently merged in to dev), and I added an extra Property Pane control to the testing web part so that feature can be tested too. edit: oh, also had to update a few references for OUIFR to @fluentui/react etc. |
Just to note, I think my changes possibly also fix the issue resolved in / raised in PR #1662 - I changed the way values were tracked in the form with this PR, So I think messages shouldn't be getting displayed inappropriately as described in that PR. If you'd prefer to merge that one first and for me to resolve conflicts again - I'm happy to do so - but equally you might decide the changes in it aren't necessary. Just thought it worth flagging. |
Background
Back in July I had this exchange with @PaoloPia (while also mentioning @AJIXuMuK ) on Twitter
https://twitter.com/T0mGerman/status/1683640358972841985
Since then, I've found some time to implement the feature discussed. It has meant adding quite a bit of code and refactoring, so I understand if this needs more of a thorough review than usual.
This update already contains the fix I proposed in #1605, but I can see other PRs for Dynamic Form - #1662 and #1625 : the result of these PRs if accepted would need to be merged with these changes, and an automatic merge is likely not possible. Happy to do a manual one though if this one is accepted.
What the PR adds
File Changes
DynamicForm.tsx
getFieldInformations
is nowgetListInformation
getListInformation
uses a new method inSPService
to get list data usingRenderListDataAsStream
. PassedRenderOptions = 64
this method returns aClientFormSchema
which includes everything we need to render custom formatting and perform client validation. It includes all field information too, in field order. The response from this endpoint does lack certain information for Number columns and server side column validation, which we fetch in a secondary call.useFieldValidation
tofalse
componentDidUpdate
)Supporting files for this functionality are in:
src/common/utilities/FormulaEvaluation.ts
src/common/utilities/CustomFormatting.ts
Tests for the formula evaluation util are in:
tests/utils/formulaevaluation.test.ts
IDynamicFieldProps and DynamicField.tsx
IDynamicField
props and changed how some are used byDynamicField
in order to cleanup legibility, adding JSDoc notation to many of them, which will (hopefully) make the purpose of each easier to understand.value
,newValue
,fieldDefaultValue
, andchangedValue
. This appeared to be one too many to me, and in some cases we were mutating prop values and using them in a way that wasn't clear. Therefore, I have reduced the value props to three:value
which is used for storing the field value from a loaded ListItemnewValue
which is used for storing values the user has actually updated since loadingdefaultValue
which is used for storing a reference to the configured default value that was set on the field or columnDynamicField
to ensure that if a component has mutually exclusive defaults, selected keys or values, that we are using the latest value entered by the user.DynamicField.tsx
has been modified to allow the pass through of error messages from validation carried out onDynamicForm
ControlsTestWebPart
.gitignore and VSCode launch config example
.vscode\Example-launch.json
and.vscode\Example-tasks.json
.launch.json
andtasks.json
respectively, then launch Debugging from within VSCode.Testing the new functionality
[$Choice] == 'Choice 2'
.If you spot any potential issues or anything - happy to discuss further and make changes.
Tom