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

Automated Browser and Repeater Fields #400

Merged
merged 11 commits into from
Nov 19, 2019
Merged

Automated Browser and Repeater Fields #400

merged 11 commits into from
Nov 19, 2019

Conversation

yanhao-li
Copy link

SAY GOODBYE TO THE LONG-WINDED updateBrowser, getFormFieldsForBrowser, updateRepeater getFormFieldsForRepeater IN YOUR REPOSITORY!


Before:

image

After:

image


Changelog

  • Automated the browser and repeater fields update and render by reading two new attributes on the model $browsers, $repeaters. Avoid the extra setup by simply providing the module name, also support extra parameters as array.
  • Conflated browsers handling methods into an individual trait: HandleBrowsers
  • Updated the model.stub to include the two new attributes.

Copy link
Member

@ifox ifox left a comment

Choose a reason for hiding this comment

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

Hi @yanhao-li!

Amazing, this is looking great.

I have 1 discussion point and 1 change request:

  • discussion: should the browsers and repeaters array be provided in the repository instead of the model? I would be in favor of the repository as I feel like the model should stay agnostic regarding that.
  • change request: this automation should also take care of hydrating the model for previews using hydrateBrowser and hydrateRepeater.

@yanhao-li
Copy link
Author

@ifox Just pushed two commits to fix the things you mentioned.

@yanhao-li yanhao-li requested a review from ifox October 1, 2019 15:20
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.

2 participants