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 code examples (or add new ones) to use the new design and not the deprecated one #283

Open
ophirKatz opened this issue Nov 22, 2022 · 6 comments

Comments

@ophirKatz
Copy link

The example in the app folder of the repo showcases usage of the old version of creating ngx-sub-forms, and not the one with createForm.
This makes it harder to adopt since I don't see a guideline for best practices.
Please add another example using the new version :)

@ophirKatz
Copy link
Author

BTW - I think the example in the README also has a mistake:
image
Is the sub form supposed to be named AddressForm/AddressControl or something? Its confusing now that it is also a PersonForm

@maxime1992
Copy link
Contributor

Hello,

The example in the app folder of the repo showcases usage of the old version of creating ngx-sub-forms, and not the one with createForm.

This is half correct. Yes it does indeed has examples of the old version. But this is very much intentional.
The reason being that we wanted to ensure a smooth transition for people who wouldn't be able to migrate all their forms at once so both APIs are compatible, even though the old one is deprecated and shall be removed soon.

If we could get rid of the examples using the old API we would but this was tremendously helpful to make sure we were supporting both API without breaking changes as our e2e tests run both with the old API (based on the folder you mentioned) and the new one.

Which leads me to the other half that you may have missed: The main folder is the deprecated API, but there's also main-rewrite which has the exact same demo but with the new code only.

That said, I do agree that we should've renamed main to maybe main-deprecated or something more explicit.

Please add another example using the new version :)

As explained, refer to main-rewrite for examples with the new API

Is the sub form supposed to be named AddressForm/AddressControl or something?

Looks like it does indeed

Also, we started a PR to remove the deprecated code but it got stale and should be revived at some point, maybe for the Angular 15 upgrade

@ophirKatz
Copy link
Author

Thank you!

@ophirKatz ophirKatz reopened this Nov 23, 2022
@188599
Copy link

188599 commented Dec 12, 2022

BTW - I think the example in the README also has a mistake: image Is the sub form supposed to be named AddressForm/AddressControl or something? Its confusing now that it is also a PersonForm

This is specially confusing because it's hard to tell which component you are meant to supply with the subformComponentProviders method, which also isn't further elaborated down in the documentation.

@maxime1992
Copy link
Contributor

BTW @ophirKatz as one of the 2 things mentioned in this issue was about the old api, just FYI I've removed it recently.

Thanks @188599 for pointing that out 👍 This should indeed being fixed

@ophirKatz
Copy link
Author

@maxime1992 I saw, thanks mate!

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

No branches or pull requests

3 participants