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.

Sign up for GitHub

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

Jump to bottom

Adds object/form renderer alongside much needed fixes #466

Merged
merged 4 commits into from
Sep 12, 2017
Merged

Adds object/form renderer alongside much needed fixes #466

merged 4 commits into from
Sep 12, 2017

Conversation

sophypal
Copy link
Contributor

@sophypal sophypal commented Sep 12, 2017 •
edited
Loading

This project uses semver, please check the scope of this pr:

  • #none# - documentation fixes and/or test additions
  • #patch# - backwards-compatible bug fix
  • #minor# - adding functionality in a backwards-compatible manner
  • #major# - incompatible API change

CHANGELOG

  • Added object/form renderer for rendering forms dynamically. Custom views can now reference an object type field directly without needing to expand the view definition further.
  • Added plugins property (utilized by the form renderer) which is responsible for generating bunsen meta data.
  • Added much needed test against the regressions found that degraded the performance.
  • Updated refocus logic to focus on init (if applicable) instead of on every change.
  • Updated the redux workflow to be more compatible with the run-loop. It should prevent issues with multiple synchronous updates causing redundant onChange actions being triggered which improves performance.
  • Fixed performance issue with when renderer causing multiple onChange events on init.
  • Fixed performance issue with value propagation due to a bug in the view precomputation and dependency diffing.
  • Removed useless internal hooks. This does not alter the hook names of public interfaces but removes the duplicates used through the intermediate DOM nodes used to render a form. There are still duplicate hooks applied to the form inputs but no change has been made to those to avoid a major release.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 5b8add8 on sophypal:form-renderer into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 25006b6 on sophypal:form-renderer into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 57e9fc4 on sophypal:form-renderer into ** on ciena-frost:master**.

@sophypal
Copy link
Contributor Author

Trying to resolve testing issue with the geolocation renderer since it seems it happens only in travis.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d1f9882 on sophypal:form-renderer into ** on ciena-frost:master**.

@sophypal sophypal changed the title Adds object/form renderer alongside and much needed fixes Adds object/form renderer alongside and much needed fixes [DO NOT MERGE - TESTING] Sep 12, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 655356c on sophypal:form-renderer into ** on ciena-frost:master**.

Fix performance regression and added new form renderer
4e5b4fb 
-Fixed double render with store updates
-Fixed precomputing cell configs
-Fixed double renderering issue when model/view conditions evaluate on defaults
-Fixed double onChange calls in when renderer
-Fixed infinite class name binding
-Rewrote focusing logic to work in Ember way reducing re-layout per change
-Added documentation for form renderer
-Update test to work with new async rendering approach

Protect against getRootProps not being defined

Add additional tests and fix last dirty attribute during re-render

Remove internal hooks

Initial integration tests for form input

Fixing lint

Address testing issues in Firefox

Fix geolocation tests
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling bc33fa0 on sophypal:form-renderer into ** on ciena-frost:master**.

@sophypal sophypal changed the title Adds object/form renderer alongside and much needed fixes [DO NOT MERGE - TESTING] Adds object/form renderer alongside and much needed fixes Sep 12, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 4e5b4fb on sophypal:form-renderer into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dd99eb9 on sophypal:form-renderer into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9699d19 on sophypal:form-renderer into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ddad794 on sophypal:form-renderer into ** on ciena-frost:master**.

@gknoy
Copy link
Contributor

gknoy commented Sep 12, 2017

@sophypal This looks pretty good. Thank you for explaining the usefulness of the new form renderer, and I like that using it as the default renderer for object fields means that the templates are simpler.

@sophypal sophypal changed the title Adds object/form renderer alongside and much needed fixes Adds object/form renderer alongside much needed fixes Sep 12, 2017
@cstolli
Copy link
Contributor

cstolli commented Sep 12, 2017

@spal I did look through this in some detail. I don't see anything that jumps out at me but I'm not sure I would. Test coverage is good, some nice improvements in async handling I see. I say merge it if everything is passing.

@sophypal
Copy link
Contributor Author

sophypal commented Sep 12, 2017 •
edited by cstolli
Loading

👍

Approved with PullApprove

@sophypal sophypal merged commit df26a88 into ciena-frost:master Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cstolli cstolli Awaiting requested review from cstolli

@rox163 rox163 Awaiting requested review from rox163

@notmessenger notmessenger Awaiting requested review from notmessenger

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sophypal @coveralls @gknoy @cstolli