Fire: Remove/comment dependencies on IRS (index, vehicle_reports)

[trendspotter]

PR for removal Fire Stations dependency on irs as briefly discussed on Google Groups.

• Removes the customized index with shortcuts to irs incident creation and replaces with either CMS page or Fire Station listing as other modules have.
• Commented out everything related to Vehicle Deployments report in similar fashion as police module has. The actual code for vehicle_report remained and is now unused.

[nursix]

Looks good to me. If vehicle_report is now unused, that should be indicated in its docstring.

[nursix]

...probably with a todo to clarify why it's unused, and what should be done to make it useful again (e.g. re-implementing the same based on event-module?)

[trendspotter]

Is @ToDo: Currently unused, requires deprecated irs module good enough?

[nursix]

Well, just imagine you were new to this code base, had a requirement that would suggest using this function, and would find that docstring: would it help you to understand the purpose of the function, and what you can do in order to make use of it?

If that's a yes, then it's good enough.

[trendspotter]

Don't mean to be disrespectful, but I don't have to imagine. I am new to the codebase I and have the requirement (resp. my employer has the intention) to use pretty much every functionality which has been already written. I'm chewing through the codebase since mid-June and looking at the number of ticket in our internal system, I'm barely half-way there. So yes, pretty much any comment is good enough. Definitely better than no comment at all. :)
And yes, I'm aware that this isn't how SE is intended to be used, but that's not a topic for discussion on GitHub.

Anyway, added the remark about reimplementation in event module.

[nursix]

Yup, that was exactly my point: from your perspective you can better judge what is "good enough" a comment, whilst I can only name the general requirement and make suggestions. Thus, if you deem your comment useful for others in the same situation as yours, then I'd rely on that.

[nursix]

No, that's actually meant to be NONE not None ;) However, this may be missing:

NONE = current.messages["NONE"]

[nursix]

So, maybe replace NONE with current.messages["NONE"] here.

[nursix]

Yup, better.

[trendspotter]

Can I get a hint for one extra fix, please? In the Fire Zone creation form, there is a Create Zone Type popup link. It opens and loads the popup content as expected, but it doesn't assign display: block to the form element, so the form remains invisible. Which piece of magic handles this?

[nursix]

Why is the form hidden in the first place? It shouldn't be.

[trendspotter]

Apparently all /create?format=popup forms are implicitly hidden and then shown via JS. Or at least this is what I see from my observation.

[nursix]

You're right - I think it's a timing problem with S3.popup_loaded (in S3.js). Apparently it's called before the DOM of the popup is ready, so I' trying to fix that now.

[trendspotter]

Timing seems to be OK. Strangely enough, that form simply doesn't do anything on jQuery show() method.

These don't work:

$('#' + id).contents().find('#popup form').show()
$('#' + id).contents().find('#popup form').css('display', 'block')
// ^- This is pretty much what show() does internally

This works:

$('#' + id).contents().find('#popup form').attr('style', 'display: block')

I also find strange that Fire Stations is the only module where the problem exists.

//Edit: It actually exhibits the same behavior also when directly opened in new window. Even there the $('#popup form').show() doesn't work.

[trendspotter]

OK, found the reason. The form contains a textarea with name="style". DOM unhelpfully overwrites the original style property of the form element, which is supposed to be a CSSStyleDeclaration object, with the textarea object. jQuery then tries to change the value of this.style.display which doesn't exist anymore. Now to find the easiest way out :)

[nursix]

Well,

this took me a while to grasp...although in hindsight it's plain obvious:

The name-attributes of input elements in a form set properties for the DOM Element. E.g. if you have an input with the name "example", then the form Element has a property form.example.

So far, so good. And when you now have an input with the name "style", then that would obviously become form.style.

But that overwrites the Element.style property of the form - which is just exactly the handle that is needed for JavaScript to set Element.style.display='block'.

And therefore, jQuery.show for the form will never succeed as long as there is a field "style" in that table.

It would of course work if, instead of having a global CSS #popup form {display:none} in widget.css, we would add a class "hide", and then remove that class in S3.popup_loaded. That'd be one way to fix it - although it would still leave the form's style inaccessible for JavaScript.

The other way to fix it is to avoid fields with the name "style" (and thereby inputs with name="style"). This is what I did for this case now, for one, because "mapstyle" is more self-explaining anyway, and for another, the field is currently unused.

However, there are other tables with fields "style" (s3db/gis.py), and those can not easily be changed in this way. Nor is it so striking obvious that you shouldn't use "style" as a field name or why.

So I think, this needs yet another round of tweaking to make it more robust - namely, to not hide/show the popup form, but rather the form container. That though requires it to be present in all templates, and in all templates equally, so I need to investigate that first.

Hence, for now, the fix:
nursix/eden@a9c2630
(Note that it's not minified yet, since I'm looking to tweak further)

[nursix]

So...now I additionally changed it to initially hide the container DIV rather than the FORM, because DIVs don't have that vulnerability.
nursix/eden@73fcee5#diff-8191c7ac5c7dc0f5704f2b8a3879305fL279
With that, the modals should now work even when there is a "style" field in the form - it would still break the form.style property, but not prevent the showing of the popup contents.

It's not the easiest change as touching widgets.css always requires to re-minify each and every theme. I already did a bunch, but I'm growing very tired now, so need to leave the rest for tomorrow to avoid mistakes.

Can you confirm whether that solves the problem?

[nursix]

Ah, you found the root cause before me - saw that only now! (sorry it's really late...)

[trendspotter]

I confirm that the Fire Station popup is now popping up correctly along with all the others :) Nice job.
The popup is missing a title however, but I know ho to fix that one, so let's first merge your changes and I'll rebase mine afterwards.

[trendspotter]

Rebased and squashed.