liquidvotingio / decidim-module-liquidvoting

GNU Affero General Public License v3.0
4 stars 0 forks source link

Enforce choosing a delegate when click 'Delegate Support' #109

Closed jinjagit closed 3 years ago

jinjagit commented 3 years ago

Add feature: If user clicks 'Delegate Support' without selecting a specific delegate, prompt with a modal alert "Please first choose your delegate" that must be dismissed before continuing.

Relates to issue #97

jinjagit commented 3 years ago

Status:

I think I have the basic structure of this change here.

This works in Firefox and Chromium, but I need to test in other browsers (Chrome, Safari, Opera, etc.)

I also need to investigate an alternative to / workaround for event.preventDefault(); as, apparently event in this context is now deprecated, but the code fails to work on Chromium without event as part of the command.

At this point, the change is easy enough to understand by simply viewing the (minimal) changes and new code in this PR.

When I have a better idea of the outstanding issues, style choices (what language we ant to use in the prompt, etc), I will add more extensive notes / screenshots, etc.

jinjagit commented 3 years ago

Actually, this works until user successfully delegates, then un-delegates, for example - then the prompt no longer appears as expected... needs more work.

EDIT: More accurately, the prompt works as expected until any after any action which would cause an ajax page update occurs - click 'Support', 'Already Supported', 'Delegate' (when a delegate is selected), etc.

jinjagit commented 3 years ago

Screenshot: Prompt if click 'Delegate Support' without selecting a delegate: [EDIT: Updated to show new, friendlier alert message] prompt_delegate

jinjagit commented 3 years ago

This looks promising for my issues with losing the "You must choose a delegate" prompt functionality after ajax refresh: StackOverflow: reinitializing-javascript-after-ajax-refresh

jinjagit commented 3 years ago

The last commit removes code duplication as intended, but introduces a warning (browser console):

[Deprecation] Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.

My idea is to place the js code for the delegation modal in a file delegation_modal.js.es6, which is loaded in a partial _delegation_modal.html.erb, which is rendered at the end of the _delegation.html.erb partial. This means I can also use append in the update_buttons_and_counters.js.erb to re-render the _delegation_modal.html.erb, thus reloading the delegation_modal js code.

This successfully avoids the previous approach of simply copying the entire contents of the delegation_modal.js.es6 into update_buttons_and_counters.js.erb, which just feels wrong (and could become verbose as we add translation related functionality to the js code).

I have spent some time trying to research/fix the warning, and will continue to experiment. References include: https://www.programmersought.com/article/12076948859/ https://thisinterestsme.com/synchronous-xmlhttprequest/ https://code2care.org/2015/synchronous-xmlhttprequest-main-thread-deprecated-detrimental-effect-end-users-experience

EDIT: I reverted (in the following commit) to the previous method of simply duplicating the JS file contents at the end of the delegation_modal.js.es6 Ajax, which works and avoids this warning.

jinjagit commented 3 years ago

TLDR: This works, but uses a duplicate of code in the JS file delegation_modal.js.es6, first loaded in the the _delegation.html.erb partial, in the update_buttons_and_counters.js.erb Ajax refresh code.

I tried many approaches to try to reload the JS file in the Ajax refresh code, but all either did not work, or resulted in the warning about not being async mentioned in previous comments.

There is a (relatively complex) modal template pattern used in Decidim core, which I haven't understood fully. However, I didn't find any usage of these modals that matched our use case (call modal if form submit contains certain form attribute value, &/or refresh the check for modal condition in Ajax refresh). Most (?all) are either opened via an explicit link (e.g. an info. modal) or due to a timeout (e.g. re-login prompt).

davefrey commented 3 years ago

Congrats for finding your way through that, even if there's a console warning it's a much better UX and system behavior