Modify

Opened 20 months ago

Closed 18 months ago

Last modified 18 months ago

#10657 closed enhancement (fixed)

Hide the 'Order' column

Reported by: rjollos Owned by: rjollos
Priority: normal Component: AdminEnumListPlugin
Severity: normal Keywords:
Cc: jun66j5 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 jun66j5 19 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 20 months ago by rjollos

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

comment:2 Changed 20 months ago by rjollos

  • Cc jun66j5 added
  • Owner changed from nonplus to rjollos
  • Status changed from new to assigned

comment:3 in reply to: ↑ description ; follow-up: Changed 20 months ago by rjollos

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 20 months ago by rjollos

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 follow-up: Changed 20 months ago by rjollos

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 20 months ago by rjollos

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 20 months ago by rjollos

(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 20 months ago by rjollos

(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 19 months ago by jun66j5

comment:9 Changed 19 months ago by jun66j5

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 19 months ago by rjollos

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

comment:11 Changed 19 months ago by rjollos

(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 19 months ago by rjollos

Thanks for the patch Jun!

comment:13 Changed 19 months ago by rjollos

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

comment:14 follow-up: Changed 19 months ago by rjollos

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 19 months ago by jun66j5

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 18 months ago by rjollos

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:17 Changed 18 months ago by rjollos

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

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from rjollos. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.