-
Notifications
You must be signed in to change notification settings - Fork 3
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
Uplift to use cht-form web component #245
Conversation
Everything should be working at this point
Allow forcing though!
# Conflicts: # dist/form-host-cht-core-4-0.dev.js # docs/core-adapter.js.html # docs/form-host_form-filler.js.html # docs/harness.js.html
@kennsippell please add/redirect reviewers for this PR as makes sense! This is not particularly urgent as it depends on the as of yet unreleased cht-core |
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 love this change
Co-authored-by: Kenn Sippell <[email protected]>
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.
sorry this review took so long... this is looking great!
# Conflicts: # dist/all-chts-bundle.dev.js # dist/all-chts-bundle.dev.js.map # dist/form-host-cht-core-4-0.dev.js # docs/Harness.html # docs/core-adapter.js.html # docs/dev-mode_mock.cht-conf.contact-summary-lib.js.html # docs/dev-mode_mock.cht-conf.nools-lib.js.html # docs/dev-mode_mock.rules-engine.rules-emitter.js.html # docs/form-host_form-filler.js.html # docs/global.html # docs/harness.js.html # docs/index.html # docs/jsdocs.js.html # docs/mock.cht-conf.module_contact-summary-lib.html # docs/mock.cht-conf.module_nools-lib.html # docs/mock.rules-engine.module_rules-emitter.html # docs/module-core-adapter.html
@kennsippell this should be ready to merge/release at your convenience! It looks like the release-notes have not been updated in awhile. I could not find an automated way to generate anything, so I made an attempt to manually pull something together based on the git commit history and the changes to the version in the package.json. I doubt it is perfect, but probably better than nothing. |
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
So, let me start with a profuse apology for the size of this PR. There just was not really any way to do this incrementally since this is basically shifting some fundamental things with regard to how the cht-core resources are consumed within the Test Harness.
I am very aware that I have made a ton of design decisions (big and small) all on my own here that may or may not be acceptable to the project maintainers. 😓 I consider this PR to be more of a "production ready" proof-of-concept to show how the integration could be done. I am exploring a lot of new ground here, and have something that seems to work pretty well, but I welcome any and all ideas on better ways to approach this!
Overview
The goal of this PR is to support depending on the latest version of code from cht-core by leveraging the new cht-form functionality for rendering forms. (Previously we were blocked from depending directly on logic in
cht-core/webapp
since it is TS code written for Angular. The cht-form web component allows us to extract this logic into html/js/css resources that the harness can depend on directly.) As a part of adjusting the dependency approach, I have sought to bring back the ability to target multiple different versions of cht-core code from the same version of the Test Harness (though this will only be available for future versions of the CHT).Build tree
I have created this diagram to try and make it easier to understand how the various pieces and files fit together. Everything in this diagram outside the
build
directory gets committed to the repo. Thedist/form-host.dev.js
and thedist/all-chts-bundle.dev.js
files are rebuilt every time thebuild.sh
script is run, but the various artifacts inside thedist/cht-core-x-x
directories are only (re)built as needed.Details
package.json
(get errors when doingnpm install
). This is one of the reasons I switched to manually cloning the repo.cht-core-bundle.dev.js
files was to avoid having to rebuild all CHT versions (contained in theall-chts-bundle.dev.js
) just to add/update one of them. I have mild hopes that this might allow for longer-term support in the harness for particular CHT versions.dist/cht-core-x-x
directories are strictly necessary for the Test Harness to function (though removing them may affect how forms are displayed in the Project Explorer). Those are getting pulled over automatically by Webpack due to their usage in the css, but they should be pretty easy to exclude if we wanted to...form-filler.js
file where I changed the logic to be less tightly coupled to internal Enekto code and more able to just directly consume the content on the page.I am pretty sure this closes #138 and closes #204 (I guess this is as close as we get to
4.1
). It might also make sense to say this addresses #46 as well since basically all the Enekto widgets should be functional at this point except for the db-object-widget (which is tracked separately in #184)...