Issue status update for http://drupal.org/node/11927 Project: Drupal -Version: 4.5.0 +Version: cvs Component: theme system Category: bug reports Priority: normal -Assigned to: stefan nagtegaal +Assigned to: Robin Monks Reported by: stefan nagtegaal Updated by: Robin Monks -Status: active +Status: patch Attachment: http://drupal.org/files/issues/tablesort_header.patch (1.49 KB) Here is a new patch with theme_tablesort_header($style) that takes either asc or desc as arguments. Tested to work on Drupal CVS HEAD. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ October 22, 2004 - 16:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement.patch (1.25 KB) Attached is a patch for tablesort.inc much cleaner, friendlier and more consistent way to theme the tablesort icons. Marking as a bug becuase it is inconsistent with the rest of drupal.. Please review and apply! ------------------------------------------------------------------------ October 22, 2004 - 16:57 : stefan nagtegaal Setting status to 'Patch' instead of 'Fixed' so it shows up in the patch queue ------------------------------------------------------------------------ October 23, 2004 - 16:27 : Dries Please move the proposed theme functions to includes/theme.inc (so Doxygen includes them in the list of theme_ functions). ------------------------------------------------------------------------ October 23, 2004 - 20:39 : Steven We should avoid entities like in the code. This can be achieved through CSS instead in the theme. ------------------------------------------------------------------------ October 23, 2004 - 20:41 : Steven (ack submitted too early) Also, I wonder about the granularity of these theme functions. I don't see the point in putting ascending/descending in a separate function: they are different styles for the same element (the tablesort indicator). ------------------------------------------------------------------------ October 24, 2004 - 00:08 : Dries Stefan's patch actually removes the entities, not (or is my jet lag worse than it appears)? ------------------------------------------------------------------------ October 24, 2004 - 10:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement_0.patch (1.73 KB) Updated patch which removes the and moves the theme_tablesort_asc() and theme_tablesort_desc() to theme.inc. This is much better imo than the current approach.. ------------------------------------------------------------------------ October 24, 2004 - 15:10 : Steven Yeah the entity was actually a +1... I should stop doing so much things at the same time. My other concern is still valid though: theme functions should theme page /elements/, not versions of that element. Otherwise we would have theme_node(), theme_node_sticky(), theme_node_teaser(), ... ------------------------------------------------------------------------ November 15, 2004 - 12:25 : Dries I agree with Steven. We should have a function called theme_tablesort_header($style) where $style can be either 'ascending' or 'descending'.