search

aurovrata / cf7-grid-layout

A WordPress plugin extension for Contact Form 7 to design responsive grid-layout forms.
https://wordpress.org/plugins/cf7-grid-layout/
GNU General Public License v2.0
6 stars 7 forks source link

Fix two "bugs" on JS toggling. #4

Closed Birmania closed 6 years ago

Birmania commented 6 years ago

First bug : On a form with more than one togglable section, the function that allow to disable fields and refresh button state was mapped multiple times to the "form.click". This results in inconsistent state of inputs fields inside the section. Ex: An opened section with disabled fields. Solution : Move the "form.click" event link just after the foreach loop in order to attach once.

Second bug : Yet, the sliding of toggle button do not trigger the accordion "activate" event. Solution : Just set "drag: false" to avoid inconsistent state.

aurovrata commented 6 years ago

thanks for the PR, I will test and revert back :)

aurovrata commented 6 years ago

First bug : On a form with more than one togglable section, the function that allow to disable fields and refresh button state was mapped multiple times to the "form.click". This results in inconsistent state of inputs fields inside the section. Ex: An opened section with disabled fields.

this is an issue I had come across previously but which I believed resolved.

Solution : Move the "form.click" event link just after the foreach loop in order to attach once.

I am not sure if this fixes the issue, but I agree that there is no need to assign the click event multiple times. This is the same for the toggled_accordion.on('sgContentIncrease...' binding.

However, please note that you have not placed either outside in your fork, hence merging this PR does not bring the required change. I am making the changes in the main fork.

Second bug : Yet, the sliding of toggle button do not trigger the accordion "activate" event.

is this on a mobile screen?

Solution : Just set "drag: false" to avoid inconsistent state.

ok, I guess if someone complains about this on a mobile screen, we could enable it using a conditional statement to detect mobile screen in the future.

PS: Thanks for both bug fixes, I will give you credit on the plugin page itself :)

Birmania commented 6 years ago

I am not sure if this fixes the issue

I agree with you !

is this on a mobile screen?

This occurs on dekstop too. To reproduce the problem, just mouse down on the toggle bullet, drag your mouse to the opposite side and mouse release. You will see that toggle is changed without any accordion modification.

While playing with this feature, I discovered a simple way to reproduce the desynchronization between input "disabled" state and accordion/toggle state ! Just mouse down the toggle, move your mouse from 1px only then mouse release. The toggle will rollbakc to its previous position but the accordion will activate, breaking the input "disabled" state.

PS: Thanks for both bug fixes, I will give you credit on the plugin page itself :)

Thanks for this, you rocks ! 👍

aurovrata commented 6 years ago

While playing with this feature, I discovered a simple way to reproduce the desynchronization between input "disabled" state and accordion/toggle state ! Just mouse down the toggle, move your mouse from 1px only then mouse release. The toggle will rollbakc to its previous position but the accordion will activate, breaking the input "disabled" state.

interesting, it would be worth reporting to the original author of js Toggles.