-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: add row in action forms #1173
Conversation
Task linked: CU-86c097bm3 Story #3 - Add "row" layout element |
3762841
to
1953b49
Compare
Code Climate has analyzed commit 79f503b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 98.0% (98% is the threshold). This pull request will bring the total coverage in the repository to 97.5% (0.0% change). View more on Code Climate. |
return element; | ||
} | ||
|
||
allFields.push(element); |
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.
😢 Mutating function parameters. 😬
{ | ||
type: 'Layout', | ||
component: 'Separator', | ||
}, | ||
{ | ||
type: 'Layout', | ||
component: 'HtmlBlock', | ||
content: 'some text content', | ||
}, |
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.
👀
field[subElementsKey] = await handler(field[subElementsKey] || []); | ||
} | ||
|
||
private async copyFields(fields: DynamicFormElement[]) { |
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.
I think this need a little explaination.
Because,
- it copy the fields but also do crazy things (async + call handler on subfields) ?
- I don't understand the need of async in there
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.
Oh ok, you use this mechanism to do async work dropDefault (= compute default value) , dropIfs (= remove field that have if: false), ...
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.
But style copyFields should be sync.
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.
You can define two versions of executeOnSubFields
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.
- ✅ Code review 💪 (some tiny improvments will come in a refactor PR)
- ✅ Use pure functions when possible
- ✅ Performance concerns
- ✅ The PR tackle one subject only
- ✅ Automatic tests
- ✅ Unit tests
- ✅ Integration tests
- ✅ Manual tests
- ✅ PR title
- ❔ PR linked to the clickup task
# [1.9.0](https://github.com/ForestAdmin/agent-nodejs/compare/example@1.8.47...example@1.9.0) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
# [1.44.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/agent@1.43.0...@forestadmin/agent@1.44.0) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.3.17 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
# [1.50.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-customizer@1.49.0...@forestadmin/datasource-customizer@1.50.0) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.1.28](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-dummy@1.1.27...@forestadmin/datasource-dummy@1.1.28) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.9.0 🎉 The release is available on Your semantic-release bot 📦🚀 |
## [1.7.17](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-mongoose@1.7.16...@forestadmin/datasource-mongoose@1.7.17) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.3.34](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-replica@1.3.33...@forestadmin/datasource-replica@1.3.34) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.9.5](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-sequelize@1.9.4...@forestadmin/datasource-sequelize@1.9.5) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.44.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
## [1.14.21](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-sql@1.14.20...@forestadmin/datasource-sql@1.14.21) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
# [1.37.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/datasource-toolkit@1.36.0...@forestadmin/datasource-toolkit@1.37.0) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.11.12](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/forest-cloud@1.11.11...@forestadmin/forest-cloud@1.11.12) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.50.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
# [1.30.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/forestadmin-client@1.29.0...@forestadmin/forestadmin-client@1.30.0) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.3.88](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/plugin-aws-s3@1.3.87...@forestadmin/plugin-aws-s3@1.3.88) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
## [1.0.99](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/plugin-export-advanced@1.0.98...@forestadmin/plugin-export-advanced@1.0.99) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.1.28 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
## [1.2.14](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/plugin-flattener@1.2.13...@forestadmin/plugin-flattener@1.2.14) (2024-09-18) ### Features * add row in action forms ([#1173](#1173)) ([8770699](8770699))
🎉 This PR is included in version 1.7.17 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
Definition of Done
CU-86c097bm3
General
Security