fulcrologic / fulcro-rad

Fulcro Rapid Application Development
MIT License
201 stars 46 forks source link

fix: change confirm fn name to not shadow the js one #92

Closed pedrocicoleme closed 2 years ago

pedrocicoleme commented 2 years ago

The js/confirm function is undefined when in the convert-options helper function in the com.fulcrologic.rad.form namespace, and so the confirmation function does not work by default when trying to leave a form. This was noted by @tylernisonoff in https://clojurians-log.clojureverse.org/fulcro/2020-08-10/1597018953.191600 and the observations in the message are still valid with regards to the js/alert function being defined but not the js/confirm until we set again the ::form/confirm option in the form component definition with #?(:cljs js/confirm).

Based on what I could understand and test/experiment/find, the js/confirm var/declaration is being shadowed by the local confirm variable set by the when-let. A better/more detailed explanation of the problem can be found at https://gist.github.com/pesterhazy/2bd06fc6e12686e0705763988099b3c5.

This PR suggests changing the function/symbol name from confirm to confirm-fn to avoid this problem.

To reproduce the problem:

  1. using the fulcro-rad-demo code
  2. in the browser, navigate to one of the accounts
  3. change a field value without saving the form
  4. try to select one of the other menu items
  5. there won't be any confirmation box asking if we really want to leave the form

Now, using the fulcro-rad repo with the suggested patch repeat the steps 1-4 and the box will popup asking if we want to leave the form and lose the unsaved changes.

awkay commented 2 years ago

ok. wow, thanks for digging into that.