Skip to content

[Dynamic Form] Fix for 1788/1794 and add styles property for support customizing styling #1913

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

Merged

Conversation

wuxiaojun514
Copy link
Contributor

Q A
Bug fix? [x]
New feature? [x]
New sample? [ ]
Related issues? fixes #1788 , partially #1794

What's in this Pull Request?

This PR is mainly for

I have documented the new feature on DynamicForm.md and given an example how to overwrite styling

I know it is quite a big change, please let me know if anything missing.

Guidance

  • You can delete this section when you are submitting the pull request.
  • Please update this PR information accordingly. We'll use this as part of our release notes in monthly communications.
  • Please target your PR to dev branch.

@wuxiaojun514
Copy link
Contributor Author

Hi @joelfmrodrigues , could you look into it if you have some time?
This PR is for fixing two bugs (#1788 ,#1794 ) on Dynamic Form when list has enabled section layout and introduced className and styles property on this control so developer can customize the styling on it.
I also updated the related documentation for the new feature.
This PR changed quite a lot of code so it is very easy to have conflict with other submission ( I already fixed the conflicts two times), so I hope it can be reviewed early...
Please contact me if anything unclear or I need modify it.

@Marinofull
Copy link

Hi @joaojmendes any updates here?

@CodingSinceThe80s
Copy link

Hi all, just wanted to add my feedback: this would be a very useful feature so looking forward to it 👍

@joelfmrodrigues
Copy link
Collaborator

@wuxiaojun514 apologies for the delay, I have seen the email notification when you tagged me a while ago but things have been a bit chaotic (in a good way 😇 ) at my end and I have been unable to look into it

@wuxiaojun514
Copy link
Contributor Author

Hi @joelfmrodrigues , never mind, just take your time 😄

Copy link
Collaborator

@michaelmaillot michaelmaillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @wuxiaojun514, this is a great improvment, thanks!

I made some suggestions and one issue when trying to test on my side, please have a look and let me know.

subComponentStyles:{
fieldStyles:{
errormessage:{
"font-size":"18px" //overwrite the error message font size in Dynamic Field
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to make this work. I tried it with a required Lookup field, without success:

image

Did I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work on my side as well. Currently subComponentStyles is not working. I will fix it and let you know

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelmaillot I have fixed it now.
I also updated the sample in documentation https://github.com/wuxiaojun514/sp-dev-fx-controls-react/blob/fix-for-1788-and-add-styles-property/docs/documentation/docs/controls/DynamicForm.md#how-to-use-styles-property
to handle normal text field (the error message in normal text field is generated inside the fluent ui TextField control, so we need use selectors to overwrite it.

          styles={{
            sectionFormField: {
              selectors: {
                ':has(div)': {
                  [`@media (min-width: 1280px)`]: {
                    "min-width": '21%',
                    "max-width": '21%' //force show 4 columns per row in big screen size when enabled list custom formatting
                  }
                },
              },
            },

            subComponentStyles: {
              fieldStyles: {
                errormessage: {
                  "font-size": "18px" //overwrite the error message font size in Dynamic Field for Lookup/Note/Date fields (The error message element is generated directly in Dynamic Field control)
                },
                fieldDisplay: {
                  selectors: {
                    '.ms-TextField-errorMessage': {
                      "font-size": "18px !important" //overwrite the error message font size in Dynamic Field for TextField (error message element is wrapped in Fluent UI TextField control, we cannot modify it directly,we have to use 'selectors' to change it)
                    }
                  }
                }
              }
            }
          }}

@wuxiaojun514
Copy link
Contributor Author

Hi @michaelmaillot
Much thanks for your review.
I appreciate your guide and testing!
Now I have updated the documentation with your advice and I have fixed typo issues in the code.
Please let me know if anything else needs fixing.

Copy link
Collaborator

@michaelmaillot michaelmaillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried again and I confirm it works now!

Just a few updates and we'll be good!

In this example it shows 4 columns (by default it shows 3 columns per row) in one row if screen size is bigger than 1280px (it requires enable list formatting first)
and make the error message font size a bit large.
```TypeScript
styles={{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove all the indent made on the code block:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, I removed all indent on this sample code.

fieldLabel:IStyle;
labelContainer:IStyle;
pickersContainer:IStyle;
FieldEditor:IStyle;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be written like this?

Suggested change
FieldEditor:IStyle;
fieldEditor:IStyle;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have corrected the class name and related document (Actually I copied this name from old DynamicForm.module.scss
file...)

@michaelmaillot
Copy link
Collaborator

Hey @wuxiaojun514,

Thanks for the updates, just this last one remaining update and we'll be good for the merge: https://github.com/pnp/sp-dev-fx-controls-react/pull/1913/files#r1993102589

@michaelmaillot michaelmaillot merged commit 29e3e98 into pnp:dev Mar 17, 2025
1 check passed
@michaelmaillot
Copy link
Collaborator

Merged manually.

Thank you!

@michaelmaillot michaelmaillot added this to the 3.21.0 milestone Mar 17, 2025
michaelmaillot added a commit that referenced this pull request Mar 17, 2025
@michaelmaillot michaelmaillot mentioned this pull request Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants