Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fabrikmodalrepeat.js breaks ajax repopulation of dropdowns #1961

Open
Sophist-UK opened this issue Feb 8, 2018 · 3 comments
Open

fabrikmodalrepeat.js breaks ajax repopulation of dropdowns #1961

Sophist-UK opened this issue Feb 8, 2018 · 3 comments

Comments

@Sophist-UK
Copy link

I am creating new backend functionality for fabrik (a visualization).

I am using fabrikmodalrepeat with a listfields inside, the list element being populated from a fabriktables field by ajax.

When you change the fabriktables selection, ajax works fine when page is first loaded.

But once you open and close the fabrikmodalrepeat, then changing the fabriktables selection creates a JS error. I have diagnosed why this is happening, but this code is too sensitive for me to touch to fix this.

What is happening is as follows:

  1. The initial listfields HTML is a select field with the listfields id, inside a table inside a hidden div with the nested fieldset id. Immediately following the hidden div is the fabrikmodalrepeat button.

  2. When you change the selection in the fabriktables field, ajax can find and repopulate a select with the field's id.

  3. When you open the fabrikmodalrepeat, the code in fabrikmodalrepeat.js removes the HTML elements from inside the hidden div and creates a new div at the bottom of the page. The original select and field id has now gone - and there is no element with the correct id, but it is not a problem at this stage because, being modal you cannot change the fabriktables selection. If you add rows inside the modal, they have repeat-group ids not the original id.

  4. When you close the fabrikmodalrepeat, the code fabrikmodalrepeat.js hides and moves the modal HTML to immediately after the control-group div (which seems odd to me rather than being inside or inside the fieldset div where it started) and also inserts hidden input fields after the fabrikmodalrepeat button with the original id containing repeat-data arrays as values.

  5. Now when you change the fabriktables selection and try to repopulate the select with the fields id, the field with that id is now an input field rather than a select field. Hence the js error.

If you open a page which uses fabrikmodalselect (say the List Module admin page which has a few of them), and open developers tools in your browser, you can see these changes happen as you open and close the fabrikmodalrepeat.

Opening it and closing it further times seems to work the majority of the time.

The biggest issue is that of ids, but additionally it seems silly to keep moving HTML around. It would seem more sensible to me to:

a. Create the original HTML inside the hidden fieldset div with a repeat group id from the start (though you have to deal with issues of zero entries.

b. Keep the HTML for the fabrikmodalrepeat popup in the same place and just show or hide it.

But as I said, this is too sensitive a set of code for me to try to fix - I am afraid of unintended consequences.

@Sophist-UK Sophist-UK changed the title fabrikmodalrepeat.js close does not reconstitute html correctly. fabrikmodalrepeat.js breaks ajax repopulation of dropdowns Feb 8, 2018
@Sophist-UK
Copy link
Author

On reflection it may not be as bad as this. It may simply be that when it closes it needs to put it back in the fieldset div. I will have a go at fixing it later.

@Sophist-UK
Copy link
Author

Sophist-UK commented Feb 8, 2018

Well I gave this a shot this evening, and fixed putting the saved HTML back in the same place and a couple of other minor fixes, however the issue of ajax updates to listfields has not been fixed even though I have tweaked a couple of minor bugs.

As far as I can tell there are several issues relating to this:

a. fabrikmodalrepeat.js adds a random number to the id for select fields, presumably because the author thought this was necessary to make the ids unique. However I do not think this is needed (all ids are already unique) and during diagnosis I saw examples of how ids ended up being just the underscore and random number. I have commented the code in #1960 but not removed this.

b. AFAICS, when you save multiple rows in the modal, you get rows with ids "base", "base-1", "base-2" etc. You cannot select only on [id^=base] because you might have another field "baseline" for example - so the ajax update code needs to merge selectors [id=base] and [id^=base-]. It might be simpler to always use ids with a suffix and make the starting point base-0.

c. When you close the fabrikmodalrepeat (fmr) without any rows, the table is saved without any selects. So there is nothing for ajax to update. I am not sure whether the options are saved in the JS object - this needs to be looked at. Use cases: Ajax before fmr is opened, ajax after fmr is opened and closed with no rows, ajax when fmr is closed with several rows.

d. Use case not handled, fmr has options. You change table, so elements are all different. listfields are not cleared even though elements have all changed.

@Sophist-UK
Copy link
Author

Module List is a good testing ground as it has multiple fabrikmodalrepeats each of which has listfields though these are not repeating. In this situation each listfields issues its own identical ajax requests when the fabriktables value changes which is not ideal though I am unclear how you would prevent this.

Google viz has repeating fabrikmodalrepeats and so is an alternative use case.

The visualization I am developing will eventually have both!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant