-
Notifications
You must be signed in to change notification settings - Fork 2.2k
PrimeReact support v6 #4497
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
base: main
Are you sure you want to change the base?
PrimeReact support v6 #4497
Conversation
Great work! This will be really useful for us! |
@jasny I've merged a few PRs with updates and fixes from |
packages/primereact/src/ArrayFieldTemplate/ArrayFieldTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/primereact/src/WrapIfAdditionalTemplate/WrapIfAdditionalTemplate.tsx
Outdated
Show resolved
Hide resolved
@jasny are you planning to complete this theme addition? What do you need from us, if anything to get this over the line? |
@heath-freenome Yes, definitely! Unfortunately, I've been tied up with work for the past weeks, leaving no room to work on any open-source effort. This is high on my priority list, and I'll pick it up as soon as time allows me. |
Sounds good. We'll have the first release of v6 beta hopefully out by the end of April. Since it is a new theme, we can add it in when you are ready |
143249a
to
e687f8e
Compare
@heath-freenome Can you help with the failing tests? I don't understand what needs to be fixed. |
packages/primereact/src/AutoCompleteWidget/AutoCompleteWidget.tsx
Outdated
Show resolved
Hide resolved
packages/primereact/src/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
I noticed you copied from the semantic-ui theme. I wonder if you are picking up stuff that that theme needed that yours doesn't? I added comments about them above. @jasny Also, you will need to run |
@heath-freenome Please let me know if changes are needed before merging. Btw, why isn't CI triggered? |
Because you are a first time dev, myself or somebody who is marked a reviewer for the project has to trigger it |
packages/primereact/src/ArrayFieldTemplate/ArrayFieldTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/primereact/src/WrapIfAdditionalTemplate/WrapIfAdditionalTemplate.tsx
Show resolved
Hide resolved
90fb184
to
fccb812
Compare
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.
Almost ready to merge. I'll be cutting the first v6 beta release in a few days hopefully
packages/primereact/src/ArrayFieldItemTemplate/ArrayFieldItemTemplate.tsx
Show resolved
Hide resolved
packages/primereact/src/ArrayFieldTitleTemplate/ArrayFieldTitleTemplate.tsx
Show resolved
Hide resolved
} from '@rjsf/utils'; | ||
import { Password } from 'primereact/password'; | ||
|
||
export default function PasswordWidget< |
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.
Can you add JSDoc, feel free to copy from another theme
packages/primereact/src/WrapIfAdditionalTemplate/WrapIfAdditionalTemplate.tsx
Show resolved
Hide resolved
@jasny sorry about the close. You'll have to rebase on main now and deal with the conflicts |
Add `PrimeReact Customization` to docs List `@rjsf/primereact` as them in the docs
Co-authored-by: Heath C <[email protected]>
Co-authored-by: Heath C <[email protected]>
Remove unused util functions Default form test Add grid test
Mock inject stylesheet. Mock dropdown, multiselect and slider. They don't work with jsdom. Update snapshots.
Co-authored-by: Heath C <[email protected]>
Set `id` property in `ArrayFieldTitleTemplate`
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.
Just a few little changes and we are ready to merge!
const { id, description } = props; | ||
if (!description) { | ||
return null; | ||
} | ||
return <span id={id}>{description}</span>; |
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.
Please use the new RichDescription
component in @rjsf/core
const { id, description } = props; | |
if (!description) { | |
return null; | |
} | |
return <span id={id}>{description}</span>; | |
const { id, description, registry, uiSchema } = props; | |
if (!description) { | |
return null; | |
} | |
return ( | |
<span id={id}> | |
<RichDescription description={description} registry={registry} uiSchema={uiSchema} /> | |
</span> | |
); |
@@ -0,0 +1,17 @@ | |||
import { DescriptionFieldProps, FormContextType, RJSFSchema, StrictRJSFSchema } from '@rjsf/utils'; |
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.
import { DescriptionFieldProps, FormContextType, RJSFSchema, StrictRJSFSchema } from '@rjsf/utils'; | |
import { DescriptionFieldProps, FormContextType, RJSFSchema, StrictRJSFSchema } from '@rjsf/utils'; | |
import { RichDescription } from '@rjsf/core'; |
export default function SelectWidget<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends FormContextType = any>( | ||
props: WidgetProps<T, S, F>, | ||
) { | ||
const multiple = typeof props.multiple === 'undefined' ? false : props.multiple; |
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.
This is easier to read and shorter
const multiple = typeof props.multiple === 'undefined' ? false : props.multiple; | |
const { multiple = false } = props; |
placeholder, | ||
readonly, | ||
value, | ||
multiple, |
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.
multiple, | |
multiple = false, |
multiple = typeof multiple === 'undefined' ? false : multiple; | ||
|
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.
Using the defaulting above alleviates the need for this
multiple = typeof multiple === 'undefined' ? false : multiple; |
@jasny After making the changes, I suggest rebuilding updating the snapshots |
"@rjsf/core": "^6.x", | ||
"@rjsf/utils": "^6.x", |
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.
"@rjsf/core": "^6.x", | |
"@rjsf/utils": "^6.x", | |
"@rjsf/core": "^6.0.0-beta", | |
"@rjsf/utils": "^6.0.0-beta", |
"@rjsf/core": "^6.0.0-alpha.0", | ||
"@rjsf/snapshot-tests": "^6.0.0-alpha.0", | ||
"@rjsf/utils": "^6.0.0-alpha.0", | ||
"@rjsf/validator-ajv8": "^6.0.0-alpha.0", |
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.
"@rjsf/core": "^6.0.0-alpha.0", | |
"@rjsf/snapshot-tests": "^6.0.0-alpha.0", | |
"@rjsf/utils": "^6.0.0-alpha.0", | |
"@rjsf/validator-ajv8": "^6.0.0-alpha.0", | |
"@rjsf/core": "^6.0.0-beta.4", | |
"@rjsf/snapshot-tests": "^6.0.0-beta.4", | |
"@rjsf/utils": "^6.0.0-beta.4", | |
"@rjsf/validator-ajv8": "^6.0.0-beta.4", |
Reasons for making this change
Added PrimeReact theme. PrimeReact is a popular component library for React.
Fixes #2928
PrimeReact is at version 10. A lot has changed since 2022 both v4 to v6 of rjsf and v7 to v10 of primereact. The theme was created from scratch instead of modifying the existing PR. Closes #2723
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update
to update snapshots, if needed.[ ] I've updated the changelog with a description of the PR(will be part of v6)