| Pcoombe | |
| Dec 6 2018, 12:58 PM |
| F32414762: image.png | |
| Oct 26 2020, 3:17 PM |
Hey! I found one bit of room for improvement in the *changedAmt* extra data checker - the *Other* field isn't factoring in, I think because it's input type number, not radio.
So - I don't think it is working as described (we'll still get changedAmt for people who switch from a preset to Other) - looking at dsk sm control, for ex: https://en.wikipedia.org/wiki/NASA?banner=B1819_0701_mlWW_dsk_p2_sm_template&force=1&country=US
Confirming that this is still an issue. I do not have a proposed solution yet, but wanted to note that I can recreate it.
I have a proposed solution in this banner.
You can see the diff between control desktop large and this banner. Important: this required modifying the CoreJS code, so I pasted that inline in this banner.
The basic logic:
If the user selects the Other amount input first, they must input a value for that action to be considered an amount selection. Then any other amount selection (including unfocusing the Other amount input and then changing that amount) sets the changedAmt flag.
Alternatively, if the user selects a defined amount then selects any other amount including the Other amount input/radio, the changedAmt flag is set.
@Pcoombe - could you review my JS changes? Lines 2253 - 2266 are the only changes from control.
Btw, a good way to review these frb variables is to set up live expressions in Chrome's inspector tools (Firefox may have something analagous). You can see that I did that with the relevant variables in this screenshot:
Thanks a lot @EWilfong_WMF, this fix looks great and I've added it to CoreJS.
Thanks also for the tip about live expressions, that made it much easier to test this! I'll have to remember it for the future.