Skip to content
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

Update documentation for template consolidation work #3028

Conversation

heath-freenome
Copy link
Member

@heath-freenome heath-freenome commented Aug 19, 2022

Reasons for making this change

  • Updated the 5.x upgrade guide for the work done around template consolidation
  • Updated the advanced customization guides for the work done around template consolidation (slightly incomplete)
  • Updated the Form properties for the work done around template consolidation
  • Updated the UiSchema docs for the work done around template consolidation
  • Updated the Utililty function documentation for the work done around template consolidation
  • Updated the array documentation for work done around template consolidation
  • Updated the CHANGELOG.md to remark about issues fixed by this release

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@heath-freenome heath-freenome force-pushed the update-documentation-for-templates branch from 5916b68 to ee242ad Compare August 23, 2022 00:07
@heath-freenome
Copy link
Member Author

@nickgros @epicfaace I believe I have updated all of the documentation. I may push a few more changes for the CHANGELOG.md but that's about it

```jsx
const uiSchema = {
title: {
"classNames": "my class"
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a big breaking change and to make it less painful to users to upgrade, what about just showing a warning instead of ignoring classNames completely? For example, I use rjsf in an upstream library where I store tons of form schemas / uiSchemas in a database. It's not feasible for me to rename classNames in all of them to ui:classNames.

The fundamental issue is that this breaking change is not as easy as just changing code, but could potentially require large data migrations, which is much more cumbersome. I wonder if many others would have a similar issue which would preclude them from upgrading to v5.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do those same schemas you have tucked away also use enumNames because those are being removed too :(

```jsx
const uiSchema = {
title: {
"classNames": "my class"
Copy link
Member

Choose a reason for hiding this comment

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

For clarity, we should provide an example with ui:classNames in it

Copy link
Member Author

Choose a reason for hiding this comment

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

If you look at the top of the file you'll see that I do provide ui:classNames in the main example of equivalence. Is that sufficient?

@heath-freenome heath-freenome force-pushed the update-documentation-for-templates branch 2 times, most recently from fea38c0 to cef4037 Compare August 25, 2022 17:23
@heath-freenome heath-freenome force-pushed the update-documentation-for-templates branch from cef4037 to 402dfec Compare August 26, 2022 21:10
@heath-freenome
Copy link
Member Author

@epicfaace @jacqueswho @nickgros I've resolved all conflicts and improved a few things. Please review ASAP so that I can begin the process of merging the rjsf-v5 branch to master to cut the release

@heath-freenome
Copy link
Member Author

@epicfaace If you'd rather have a little more time to review things, might I suggest letting this PR merge and following it up with an update in a later PR... It is a beta so things will take time to gel.

Copy link
Contributor

@nickgros nickgros left a comment

Choose a reason for hiding this comment

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

Haven't reviewed in depth, approving so we can merge. Will take a closer look before we exit beta

- Updated the 5.x upgrade guide for the work done around template consolidation
- Updated the advanced customization guides for the work done around template consolidation (slightly incomplete)
- Updated the Form properties for the work done around template consolidation
- Updated the UiSchema docs for the work done around template consolidation
- Updated the Utililty function documentation for the work done around template consolidation
- Updated the array documentation for work done around template consolidation
@heath-freenome heath-freenome force-pushed the update-documentation-for-templates branch from 402dfec to 20d7008 Compare August 27, 2022 00:38
@heath-freenome heath-freenome merged commit 21ba532 into rjsf-team:rjsf-v5 Aug 27, 2022
@heath-freenome heath-freenome deleted the update-documentation-for-templates branch August 27, 2022 01:02
heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this pull request Aug 27, 2022
* Update documentation for template consolidation work
- Updated the 5.x upgrade guide for the work done around template consolidation
- Updated the advanced customization guides for the work done around template consolidation (slightly incomplete)
- Updated the Form properties for the work done around template consolidation
- Updated the UiSchema docs for the work done around template consolidation
- Updated the Utililty function documentation for the work done around template consolidation
- Updated the array documentation for work done around template consolidation

* - Additional documentation

* - More changes for CHANGELOG.md as I review issues

* - Made the `classNames` change a deprecation instead of a break
- Restored description of `RJSFSchema`

* - Additional cleanup of documentation after closer self-review
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.

3 participants