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

ChangesetForm: Only allow changeset as argument, removing auto mode #124

Merged

Conversation

betocantu93
Copy link
Contributor

@betocantu93 betocantu93 commented Dec 2, 2020

ChangesetForm Refactor

This PR aims to provide more simple yet flexible api for ChangesetForm component.

The component serves as a lightweight mechanism for common tasks, like preventing browser native event reload on submit, persisting or rolling back changes to the changeset and providing contextual components with some predefined args like @changeset to all types of inputs, and so, preventing typos and boilerplate needed in order to support ChangesetForm Fields

It builds on top of a new utility/ergonomic component called <ChangesetForm::Context /> which serves as a way to pass down common args like the changeset to all of the types of inputs. This allows the client to optionally use it to build custom forms without much hassle.

This PR also introduces a new feature and a breaking change.

Feature:

  1. You can now pass new changesets for completely reset the form.

Breaking change:

Previously we had two "modes":

  1. Manual mode: passing down a changeset to the component via @changeset
  2. Auto mode: passing down an object or so via @model and optionally @validations

Auto mode is deprecated, now you must provide a changeset to the component, you can easily do it by using the changeset helper

Before:

<ChangesetForm @model={{this.model}} @validations={{this.validations}} />

After:

<ChangesetForm @changeset={{changeset this.model this.validations}} />

Copy link
Owner

@josemarluedke josemarluedke left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.

This is an interesting use case. I think with a few changes, you can use the changeset args.

@@ -0,0 +1,15 @@
{{yield
Copy link
Owner

Choose a reason for hiding this comment

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

I think you need a correspondent TS/Js file here, I'm not sure if template-only components work in addons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

@josemarluedke josemarluedke left a comment

Choose a reason for hiding this comment

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

Changes are looking real good!

Can you also update the docs demos to match this refactor?

@josemarluedke
Copy link
Owner

Looking good! Let's try to get CI green, then we should be ready to ship.

@josemarluedke josemarluedke changed the title allow the changeset to be replaced ChangesetForm: Only allow changeset as argument, removing auto mode Dec 19, 2020
@josemarluedke josemarluedke merged commit dcd32c9 into josemarluedke:master Dec 19, 2020
@josemarluedke
Copy link
Owner

Thank You @betocantu93 for your work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants