Clicking outside open Editor form can deselect table rows: expected behavior, or bug?

MichaelCarty

In my setup, I've initialized the DataTable with select: { blurable: true } and the Editor with formOptions: { main: { onBackground: "none" } }, as I like being able to "click off" selected rows to deselect them, but also prefer the Editor not to close if I click the outside background overlay (annoying when several fields have been edited but an errant outside click accidentally closes the modal before submission). However, when I use these two options in tandem, if I have the Editor open and click outside the form but over a selected table row, the row deselects. From a UX perspective this feels wrong to me, as I'd expect that darkened area to be "dead."

This example is modeled after the Editor "Basic initialization" one, but with "blurable" and "onBackground" set per my use case. Select, for example, the first table row, then click Edit, then click somewhere outside the form but over the table's selected row:
http://live.datatables.net/xutodizi/2/edit

To achieve the behavior I prefer, I'm currently patching this part of dataTables.select.js (@ line 434 in v1.3.1) ...

// Don't blur in Editor form
if ( $(e.target).parents('div.DTE').length ) {
    return;
}

... with this ...

 // Don't blur when Editor's open
if ( $(e.target).closest('div.DTED_Lightbox_Background, div.DTED').length ) {
    return;
}

... so that the table blur trigger is avoided not just when clicking things on the Editor form itself, but also when clicking any outside area (all of that being covered by DOM siblings div.DTED_Lightbox_Background and div.DTED).

Earlier example, but with my current patch:
http://live.datatables.net/xutodizi/3/edit

Would the original behavior be considered expected, or a bug?

allan

Excellent point - thank you for posting this!

I think the change shoudl actually go into Editor's lightbox code - you'll find:

        dom.background.bind( 'click.DTED_Lightbox', function (e) {
            self._dte.background();
        } );

        $('div.DTED_Lightbox_Content_Wrapper', dom.wrapper).bind( 'click.DTED_Lightbox', function (e) {
            if ( $(e.target).hasClass('DTED_Lightbox_Content_Wrapper') ) {
                self._dte.background();
            }
        } );

that should become:

        dom.background.bind( 'click.DTED_Lightbox', function (e) {
            e.stopImmediatePropagation();
            self._dte.background();
        } );

        $('div.DTED_Lightbox_Content_Wrapper', dom.wrapper).bind( 'click.DTED_Lightbox', function (e) {
            if ( $(e.target).hasClass('DTED_Lightbox_Content_Wrapper') ) {
                e.stopImmediatePropagation();
                self._dte.background();
            }
        } );

And it will in the next release .

Allan

MichaelCarty

Ah, cool. It does make more sense to handle it from that direction. I've updated my temporary patch accordingly. Thank you, Allan!

MichaelCarty

Oh, just realized this though @allan, as I'm now dabbling in Bootstrap: would the solution be different for the Bootstrap version of Editor, given that those lightbox handler bindings reference classes that apparently aren't on the Bootstrap versions of the DOM elements? If those event handlers are being bound to the Bootstrap versions, I can't seem to find where.

allan

Hmm - that's a good point. Bootstrap has its own event handlers for its background click, so it might need a workaround in Select for that. I'll need to have a think about it.

Allan