-
Notifications
You must be signed in to change notification settings - Fork 409
New Dynamic Form Feature(s) - Custom Formatting and Validation, ControlsTestWebPart updates #1672
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
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
getFieldInformationsis nowgetListInformationgetListInformationuses a new method inSPServiceto get list data usingRenderListDataAsStream. PassedRenderOptions = 64this method returns aClientFormSchemawhich 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.useFieldValidationtofalsecomponentDidUpdate)Supporting files for this functionality are in:
src/common/utilities/FormulaEvaluation.tssrc/common/utilities/CustomFormatting.tsTests for the formula evaluation util are in:
tests/utils/formulaevaluation.test.tsIDynamicFieldProps and DynamicField.tsx
IDynamicFieldprops and changed how some are used byDynamicFieldin 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:valuewhich is used for storing the field value from a loaded ListItemnewValuewhich is used for storing values the user has actually updated since loadingdefaultValuewhich is used for storing a reference to the configured default value that was set on the field or columnDynamicFieldto 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.tsxhas been modified to allow the pass through of error messages from validation carried out onDynamicFormControlsTestWebPart
.gitignore and VSCode launch config example
.vscode\Example-launch.jsonand.vscode\Example-tasks.json.launch.jsonandtasks.jsonrespectively, then launch Debugging from within VSCode.Testing the new functionality
[$Choice] == 'Choice 2'.{ "elmType": "div", "attributes": { "class": "ms-borderColor-neutralTertiary" }, "style": { "width": "99%", "border-top-width": "0px", "border-bottom-width": "1px", "border-left-width": "0px", "border-right-width": "0px", "border-style": "solid", "margin-bottom": "16px" }, "children": [ { "elmType": "div", "style": { "display": "flex", "box-sizing": "border-box", "align-items": "center" }, "children": [ { "elmType": "div", "attributes": { "iconName": "Group", "class": "ms-fontSize-42 ms-fontWeight-regular ms-fontColor-themePrimary", "title": "Details" }, "style": { "flex": "none", "padding": "0px", "padding-left": "0px", "height": "36px" } } ] }, { "elmType": "div", "attributes": { "class": "ms-fontColor-neutralSecondary ms-fontWeight-bold ms-fontSize-24" }, "style": { "box-sizing": "border-box", "width": "100%", "text-align": "left", "padding": "21px 12px", "overflow": "hidden" }, "children": [ { "elmType": "div", "txtContent": "='This is the selected user and lookup - ' + [$Person.title] + ' - ' + substring([$Lookup],indexOf([$Lookup], ';#') + 2,100)" } ] } ] }{ "elmType": "div", "style": { "width": "100%", "text-align": "left", "overflow": "hidden", "border-top-width": "1px" }, "children": [ { "elmType": "div", "style": { "width": "100%", "padding-top": "10px", "height": "24px" }, "children": [ { "elmType": "a", "txtContent": "='Contact Details for ' + [$Title]", "attributes": { "target": "_blank", "href": "='https://aka.ms/contacts?email=' + [$Person.email]", "class": "ms-fontColor-themePrimary ms-borderColor-themePrimary ms-fontWeight-semibold ms-fontSize-m ms-fontColor-neutralSecondary–hover ms-bgColor-themeLight–hover" } } ] } ] }{ "sections": [ { "displayname": "Section 1", "fields": [ "Title" ] }, { "displayname": "Section 2", "fields": [ "Person", "Lookup" ] }, { "displayname": "Section 3", "fields": [ "Choice", "Number", ] } ] }If you spot any potential issues or anything - happy to discuss further and make changes.
Tom