GSA / touchpoints

Feedback platform for continuous improvement of systems, services, processes, and policy.
https://touchpoints.digital.gov
Other
48 stars 25 forks source link

refactor js form #1437

Closed ryanwoldatwork closed 2 months ago

ryanwoldatwork commented 4 months ago

this branch is about refactoring the fba.js.erb so that the form's options are extracted into an js options object. this prevents custom logic from being written inside the form, making the form .js more standard for clients and generally, more configurable.

this branch also namespaces questions by their unique id, rather than #answer_01, which can conflict when multiple IDs are being rendered on a single page (which would happen if multiple Touchpoints are rendered to a page)

ryanwoldatwork commented 4 months ago

i started adding in a feature flag here, and not sure if its worth going all the way, or backing it out.

the way question ID selectors are handled between versions would require conditionals in many more places.

sanason commented 4 months ago

How are people adding custom logic to their forms? I'm experiencing a pretty serious failure of imagination here. They can't alter what is returned by https://touchpoints.app.cloud.gov/touchpoints/:form_id/js surely?Are they loading their own scripts, in addition to https://touchpoints.app.cloud.gov/touchpoints/:form_id/js, and using their scripts to override the behavior of the Touchpoints script?

sanason commented 4 months ago

Do you think it might be worth splitting this into 2 separate PR's with 2 separate feature flags, one for the _fba.js.erb refactoring and one for the question ID changes? It seems like the _fba.js.erb refactor has a much narrower impact on the code, so maybe that change could be pushed through immediately, without needing to wait for verification and testing of the question ID changes. And the revert surface would be smaller in case something needed to be reverted.

ryanwoldatwork commented 4 months ago

I'm inclined to leave a single PR, because:

sanason commented 4 months ago

Did you try running the HTML generated by the new branch's code through an HTML validator?

Just for kicks, I tried loading a page with two different embedded inline forms and ran the generated HTML through https://validator.w3.org/#validate_by_input. There were a few errors reported:

The relevant question types were text_field and dropdown.