Modify

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#12062 closed defect (fixed)

Doubleclick on checkbox labels doesn't work on filters added through the OR menu

Reported by: Ryan J Ollos Owned by: Matoba Akihiro
Priority: normal Component: QueryUiAssistPlugin
Severity: normal Keywords:
Cc: Trac Release:

Description

Thank you for the nice plugin. I found it very useful and I'm working on adding the features to the Trac core in trac:#11417. Please feel free to comment on that ticket. Most likely the QueryUiAssistPlugin won't be needed by Trac 1.2. One fewer plugin is always nice :)

The issue I encountered is that the double-click doesn't function on the label of a checkbox when a filter is added using the OR selector.

I set off to fix this, and ended up making a few other changes as well:

  • Fixed implicitly declared variables
  • Optimized some jQuery selectors by limiting the scope of the search
  • Some PEP-0008 changes in the Python code.
  • Use prop rather than attr since Trac 1.0 uses jQuery 1.8.

The patch is attached. I hope it doesn't seem like I'm picking at your code. I learned far more from studying it than I'm contributing with the minor list of issues above :)

Attachments (3)

t12062.diff (3.7 KB) - added by Ryan J Ollos 3 years ago.
CustomQuery.png (31.9 KB) - added by Ryan J Ollos 3 years ago.
t12062-delegated-events.diff (2.2 KB) - added by Jun Omae 3 years ago.

Download all attachments as: .zip

Change History (18)

Changed 3 years ago by Ryan J Ollos

Attachment: t12062.diff added

Changed 3 years ago by Ryan J Ollos

Attachment: CustomQuery.png added

comment:1 Changed 3 years ago by Matoba Akihiro

Status: newaccepted

Changed 3 years ago by Jun Omae

comment:2 Changed 3 years ago by Jun Omae

I think it would be simple and easy to use .on() method to handle delegated events rather than setTimeout(). See t12062-delegated-events.diff.

comment:3 Changed 3 years ago by Matoba Akihiro

Resolution: fixed
Status: acceptedclosed

In 14387:

closes #12062

comment:4 Changed 3 years ago by Ryan J Ollos

There's a minor regression in [14387]. Double-click on a checkbox still selects the checkbox and clears all the neighbours, but double-click on the label of a checkbox no longer clear all the neighbours.

  • queryuiassistplugin/1.0/uiassist/htdocs/js/enabler.js

    diff --git a/queryuiassistplugin/1.0/uiassist/htdocs/js/enabler.js b/queryuiassi
    index f0dc07e..8ecd52d 100644
    a b  
    2121  function binder() {
    2222    var $filters = $("#filters");
    2323    $filters.on('dblclick', 'label', flip);
    24     $filters.on('dblclick', ':checkbox', selectone);
     24    $filters.on('dblclick', ':checkbox, :checkbox + label', selectone);
    2525  }
    2626
    2727  $(document).ready(function() { binder() })

comment:5 Changed 3 years ago by Matoba Akihiro

Resolution: fixed
Status: closedreopened

thanks! I'll update soon...

comment:6 Changed 3 years ago by Matoba Akihiro

Resolution: fixed
Status: reopenedclosed

comment:7 Changed 3 years ago by Ryan J Ollos

One more change. Although it appears to be harmless, the flip function is executed when double-clicking on a checkbox or its label. The following fixes the issue:

  • queryuiassistplugin/1.0/uiassist/htdocs/js/enabler.js

    diff --git a/queryuiassistplugin/1.0/uiassist/htdocs/js/enabler.js b/queryuiassi
    index 8ecd52d..de5ce4c 100644
    a b  
    2020  // bind "flip" above to labels in page.
    2121  function binder() {
    2222    var $filters = $("#filters");
    23     $filters.on('dblclick', 'label', flip);
    24     $filters.on('dblclick', ':checkbox, :checkbox + label', selectone);
     23    $filters.on('dblclick', 'th label', flip);
     24    $filters.on('dblclick', 'td :checkbox, td :checkbox + label', selectone);
    2525  }
    2626
    2727  $(document).ready(function() { binder() })

The addition of the th selector is what fixes the issue. The td selectors were just added for code clarity.

comment:8 Changed 3 years ago by Matoba Akihiro

Resolution: fixed
Status: closedreopened

comment:9 Changed 3 years ago by Ryan J Ollos

New ticket for integrating this plugin to the Trac core is trac:#11970.

comment:10 Changed 3 years ago by Ryan J Ollos

I added a notice in QueryUiAssistPlugin@9. Please let us know if you have any suggestions about the changes in trac:#11970. My aim is to make this plugin no longer necessary by Trac 1.2, by having equivalent functionality in Trac.

comment:11 Changed 3 years ago by Matoba Akihiro

Glad to hear that the function is now in Trac core, thank you!

comment:12 Changed 2 years ago by Ryan J Ollos

Is it okay to commit the change in comment:7?

comment:13 Changed 2 years ago by Matoba Akihiro

yes!

comment:14 Changed 2 years ago by Ryan J Ollos

Resolution: fixed
Status: reopenedclosed

Thanks! Fixed in [14862].

comment:15 Changed 2 years ago by Ryan J Ollos

Changes in trac:#11970 have finally been committed (changeset:14225). Thanks Matobaa!

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Matoba Akihiro.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.