Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#10657 closed enhancement (fixed)

Hide the 'Order' column

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Component: AdminEnumListPlugin
Severity: normal Keywords:
Cc: Jun Omae Trac Release:

Description

When this plugin is enabled (i.e. when JS is enabled in the browser), it would be nice to hide the 'order' column since it no longer serves much purpose. With at least Trac 1.0, there seems to be a conflict: when items are reordered with the select box, the order updates correctly but it is no longer possible to reorder the items by drag and drop.

Possibly related, the following errors are seen in the JS error console on Chrome 21 when re-ordering with either the selects or by drag and drop:

Uncaught TypeError: Object function (a,b){return new p.fn.init(a,b,c)} has no method 'curCSS'

It would also be useful to provide a tooltip indicating that the items can be reordered by drag and drop.

Attachments (1)

adminenumlistplugin-r12474.diff (2.8 KB) - added by Jun Omae 4 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by Ryan J Ollos

I intend to provide a patch for this ticket and #10656 soon.

comment:2 Changed 4 years ago by Ryan J Ollos

Cc: Jun Omae added; anonymous removed
Owner: changed from Stepan Riha to Ryan J Ollos
Status: newassigned

comment:3 in reply to:  description ; Changed 4 years ago by Ryan J Ollos

Replying to rjollos:

Possibly related, the following errors are seen in the JS error console on Chrome 21 when re-ordering with either the selects or by drag and drop:

The errors are due to a compatibility issue between jQuery UI < 1.8.22 and jQuery 1.8. Since Trac provides jQuery UI 1.8.21, we'll have to implemented a workaround. I'll commit this shortly.

comment:4 in reply to:  3 Changed 4 years ago by Ryan J Ollos

Replying to rjollos:

The errors are due to a compatibility issue between jQuery UI < 1.8.22 and jQuery 1.8. Since Trac provides jQuery UI 1.8.21, we'll have to implemented a workaround. I'll commit this shortly.

This is a bit more complex than I expected, so I've opened a dedicated ticket: #10688.

comment:5 Changed 4 years ago by Ryan J Ollos

The more I think about this, we might want to have an option hide_selects to preserve the previous behavior. We'll need to pass the boolean value to the script, and in order to preserve compatibility with 0.11 we can't use add_script_data, so we'll have to follow Jun's suggestions for passing data to a script in Trac 0.11.

I haven't tested yet with Trac 0.11 though, and there doesn't seem to be any reason that this plugin should work with Trac 0.11. Trac 0.11.0 provides jQuery 1.2.3, and Trac 0.11.7 provides jQuery 1.2.6. This plugin currently packages jQuery UI 1.7.2, which requires jQuery 1.3.

I've come across situations, such as with the DateFieldPlugin, where the functionality utilized in jQuery UI just happens to work with a version of jQuery that is older than what is officially supported, so maybe this is okay. More testing is needed. If the plugin is not working with Trac 0.11, then we can just drop support for that Trac version and move on. If someone needs it later, we/they can work to make it backward compatible.

If the plugin does work with Trac 0.11, we have several options:

  • Make a release for v1.1 of the plugin and drop support for < Trac 0.12 while implementing this feature.
  • Add the option and do the work of passing the boolean value to the script and aim for full compatibility with Trac 0.11.
  • Don't add the hide_selects option because it is deemed not worth the trouble.

Feedback on these comments is welcome and appreciated.

comment:6 in reply to:  5 Changed 4 years ago by Ryan J Ollos

Replying to rjollos:

  • Add the option and do the work of passing the boolean value to the script and aim for full compatibility with Trac 0.11.

I have decided to go this route.

comment:7 Changed 4 years ago by Ryan J Ollos

(In [12462]) Refs #10657:

  • Added option to hide the column of select elements.
  • Script is only added to pages with enum panels, rather than all admin pages.
  • Bumped version to 2.0dev.
  • Updated changelog for recent changes.
  • Added 3-Clause BSD license to source.

comment:8 Changed 4 years ago by Ryan J Ollos

(In [12463]) Refs #10657, #10695:

  • closest doesn't exist until jQuery 1.3, so its use was replaced with parents.
  • prop is recommended for setting the DOM element statesdisabled and checked in jQuery 1.6+. prop is aliased to attr in version of jQuery that it doesn't exist.

In Trac 0.11.0 (jQuery 1.2.3) the indeterminate state of the checkbox is not seen. It may not be supported in earlier versions of jQuery.

Changed 4 years ago by Jun Omae

comment:9 Changed 4 years ago by Jun Omae

I tried adminenumlistplugin r12474 and found some issues.

  1. Should use BoolOption instead of Option for hide_selects. BoolOption is available since Trac 0.10.
  2. If Python 2.4, req.path_info.startswith(self._panels, ...) raises the following error.
    TypeError: coercing to Unicode: need string or buffer, tuple found
    
  3. If Python 2.4, 'true' if self.hide_selects else 'false' raises SyntaxError.
  4. If hide_selects is true with non en_US locale, Order header cell is not hidden.

Here is adminenumlistplugin-r12474.diff tested with Trac 0.11, 0.12.4 and 1.0-stable.

comment:10 Changed 4 years ago by Ryan J Ollos

(In [12475]) Refs #10657: Added COPYING file that should have been included in [12462].

comment:11 Changed 4 years ago by Ryan J Ollos

(In [12476]) Refs #10657: Use BoolOption rather than Option. Restored Python 2.4 compatibility. Hiding the order column did not work for locales other than en_US. Patch by Jun Omae (jun66j5).

comment:12 Changed 4 years ago by Ryan J Ollos

Thanks for the patch Jun!

comment:13 Changed 4 years ago by Ryan J Ollos

t:#10994 has been opened for including a reworked version of this plugin in the Trac core.

comment:14 Changed 4 years ago by Ryan J Ollos

It looks like I could do a small bit of refactoring:

$group_checkbox.prop('checked', true).prop('indeterminate', false);

->

$group_checkbox.prop({'checked': true, 'indeterminate': false});

and likewise for similar lines with multiple calls to prop.

Overall, do you think it is ready for the official release of version 2.0 Jun?

comment:15 in reply to:  14 Changed 4 years ago by Jun Omae

Sorry for my late response.

Overall, do you think it is ready for the official release of version 2.0 Jun?

Looks good to me.

comment:16 Changed 4 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

comment:17 Changed 4 years ago by Ryan J Ollos

(In [12620]) Refs #10657: Updated changelog.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.