iommirocks / iommi

Your first pick for a django power cord
http://iommi.rocks
BSD 3-Clause "New" or "Revised" License
784 stars 51 forks source link

I believe JS needs some refactoring #418

Closed berycz closed 1 year ago

berycz commented 1 year ago

1) would be nice to be able to pass parent to iommi_init_all_select2(), so it doesn't need to look for all select2's, but only inside a reloaded part

2) reloading tbody from ?/tbody endpoint isn't possible without a filter form, which would be nice. Lets say I want to reload table every n minutes, or on a websocket message and the table does not have a filter form

3) ajax_enhance.js L154 iommi_query_populate(form, container); - isnt that a "bug"? since it's defined as async function iommi_query_populate(form) {

4) would be better to put JS as static files instead of having them inside html code imo

Edit: ad 2) if filter is empty, it should take in account ?page=..., and when that's done, how about pagination would use that instead of reloading the whole page? it might be great in a Page with eg ArtistForm.edit + AlbumsTable

berycz commented 1 year ago
  1. "replace" iommi_show_spinner with events, so everybody can make their own spinners as they wish. Someone may want spinner over table and in submit button, someone may want spinner over the whole page, depends on the project . something like:
    
    element.dispatchEvent(
    new CustomEvent("iommi.loading.start", {
        bubbles: true,
        // detail: { filter: () => filter_form_element },  // for table only when filter is present, although I'm not sure if it is really needed
    })
    );

document.dispatchEvent( new CustomEvent("iommi.loading.end", { bubbles: true, // detail: { filter: () => filter_form_element }, }) );


I'd trigger it on the real element (form/table/whatever) with bubbling, that way you can listen on container, document, whatever and by `event.target` you can decide, what to do and where.
For table maybe pass `filter_form_element` or the clicked `"a_element"` for pagination? or `"a_element"` vs `"button_element"` for filter? Something like that might be useful

6. And with that I'd introduced some more custom iommi events/listeners:
for example I'd like something like `iommi.form.reloaded` and `iommi.table.reloaded` - listeners would do `iommi_init_all_select2(element);`, my `iommi_init_all_daterangepicker(element);` from #412 , etc
use case - `Form` with submit via ajax, with validation errors I return the whole form and I need to re-init all JS plugins on that form
.
another might be `iommi.error(text)` or even `iommi.message(text, type)`, so if something happens in iommi JS code (try-catch or so), anybody can make a listener that pops the message somewhere on the page
berycz commented 1 year ago
  1. to get rid of Axios and use fetch instead to cancel previous request with fetch can be done with https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal and AbortController can take care of timeouts
berycz commented 1 year ago
  1. iommiTableCall cannot be "global" var, on a Page with multiple tables one would abort another

  2. iommi_enhance_form -> onChange has to skip file-inputs, because:

    • new URLSearchParams(formData) would probably throw an error
    • iommi endpoints work only with method=GET -> so when there really is a file-input, let them add a custom onchange listener on the file-input instead
berycz commented 1 year ago
  1. iommi_validate_form throwing JS errors on field errors (fields)