Closed (fixed)
Project:
Drupal core
Version:
5.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
19 Sep 2006 at 13:32 UTC
Updated:
22 Sep 2010 at 16:15 UTC
Jump to comment: Most recent file
Comments
Comment #1
lyricnz commentedHere's a suggested design, that should degrade nicely on non-javascript users:
1) Tag some container element (p, div, th, etc) on the page in question with a unique id. This is where the "check all" control will be inserted (if javascript is enabled)
2) Add a hidden element to the form which contains the unique id. Use a fixed name for this element.
3) Once the page is loaded, look for the form fields (2), and if found insert the checkbox at place (1)
4) On click, set all the checkboxes in the form to the same state as the checkall checkbkx
This should support:
- multiple checkall per form
- multiple checkall forms per page
Comment #2
lyricnz commentedHere's a first attempt at the javascript side of above:
And changes to node.module to use this
I can think of a couple of problems straight away (putting two checkall on a single form may get itself into a loop - not tested), but the basic situation works great : if javascript is enabled, a checkbox is added to the header of the form, which (when toggled) changes the state of all the checkboxes. If javascript is not enabled, there is no change.
Comment #3
lyricnz commentedOops, my javascript got escaped. See attachment.
Comment #4
m3avrck commentedI'm a bit too tired to work on this more, but here is the code to select all checkboxes for a form, that excludes other elements.
Comment #5
edmund.kwok commentedYou can also do this to check/uncheck all checkboxes.
Check:
$(’input[@type=checkbox]’).attr('checked', 'true');
Uncheck:
$(’input[@type=checkbox]’).removeAttr('checked');
Comment #6
webchickYou can't have multiple IDs named the same on the page -- in valid XHTML, all IDs must be unique.
Could you do what you want to do by adding a class to each checkbox?
Comment #7
AmrMostafa commentedThis is a solution based on FAPI's checkboxes type. Tries to be simple.
From the user prespective, nothing changes in the UI at all. But, when they double click any of the checkboxes, all checkboxes in same group will be selected/deselected.
From a module author prespective, they will have to need to associate an extra class to the checkboxes element, and the class has to follow this format : checkboxes-$name.
As an example, here is how admin/content/node form after modification.
I also included a title => t('Double click to select/deselect all') as a try to help the user know about the feature, that's not 100% correct though, as it will make no sense to users who can't use the feature (e.g. those w/o JS). I could have injected the title by JS, but it wouldn't be localized.
Comment #8
webchickHere's a question, from someone who's completely clueless when it comes to jQuery, but knows enough JS to get by.
Wouldn't it be possible to do this behaviour on just "all" checkboxes on a form, rather than having to put a dummy class/ID at all? edkwh's snippet at #5 seems about right. Or is the reason we need a dummy class because we need to add the button dynamically only if a person has JS support?
Comment #9
AmrMostafa commentedYou are absolutely right, we were trying to solve the problem of having more than one group of checkboxes in the same form, but I quickly checked all the forms in drupal cvs, and didn't find a single case like that.
But now remains another issue, is where to insert the checkall button. I couldn't think of one elegant way to allow specifying the button location in the form without forcing extra markup to non-js clients. So I tried thinking of alternatives to a button, something myself felt comfortable with was double click. So you double click any of the checkboxes, and all other checkboxes in same group will be selected/deselected. Any ideas on this one?
Comment #10
ransom commentedSo you double click any of the checkboxes, and all other checkboxes in same group will be selected/deselected.
I know nothing.. but that sounds problimatic to me.. I imageon someone going down there module list and axadently double clicking and crying.. Plz don't make me cry? Check all & Clear all should be seperate (to protect the dumb AKA me)
Could I request a clone roll/permission button &/or ajax drag'n drop enhancment with a side of Select all?
Comment #11
lyricnz commentedRe: comment 8.
Yes, the reason I used the dummy field, was that I wanted the checkall box(s) to only be inserted if the user had javascript. One of my design goals was to have *no* impact on non-JS users. You're right though, we can't use duplicate CSS IDs, so I guess that means either a composite field #value=hole1|hole2 (ick) or two fields (double ick).
Comment #12
RobRoy commentedDude, I think I did something cool for once! Wheee! I combined some of the stuff from both patches (thanks guys) and some stuff from the jQuery docs. This is my first jQuery attempt so I'd like some feedback.
This finds any [span class="checkall-foo"][/span] and turn it into a check box that will check/uncheck all check boxes with the class "checkbox-foo" when it is checked/unchecked, respectively. Pretty unobtrusive for when JS is disabled.
I provided some use cases in this patch. They still need to be CSS aligned correctly, cleaned up (the admin -> perms is ugly I know, it's just PoC) and need some usability text so people know they will check/uncheck.
Also on the todo, maybe we should not show the checkbox when there are no results (i.e. no row checkboxes)? No biggie.
And, I didn't do modules page or blocks page as I think it seems pretty rare to enable/disable all of those at once.
Check out to see it in action...
http://example.com/admin/content/node
http://example.com/admin/content/comment
http://example.com/admin/user/user
http://example.com/admin/user/access
Comment #13
lyricnz commentedRather than modifying core, perhaps this should be a seperate module for now? Can we do everything required from a form_alter hook?
Comment #14
RobRoy commentedIMO this should be in core. This is a basic, fundamental usability enhancement and I'm surprised it hasn't gotten more attention. I think my patch is getting there. I'd just like a quick look by a higher up to know I'm on the right track and I'll finish it off.
Comment #15
kkaefer commented*subscribing*
Comment #16
RobRoy commentedOkay, I've changed it so all we include is a [span class="checkall"][/span] somewhere within the form and checking/unchecking that box will check/uncheck all checkboxes in the enclosing form. Also, removed the perms checkboxes so we just have this functionality on these pages now:
http://example.com/admin/content/node
http://example.com/admin/content/comment
http://example.com/admin/user/user
Comment #17
RobRoy commentedHere is the standard post-with-patch-after-I-forgot-to-upload-my-patch-in-the-last-post.
Comment #18
RobRoy commentedChanged two tabs to spaces in checkbox.js.
Comment #19
AmrMostafa commentedI've tested that against latest CVS, works great!
Minor thing though, the new checkbox is a bit drifted to the left (not in the same line with other checkboxes vertically). Sreenshot attached.
Comment #20
AmrMostafa commentedIMHO, this is ugly
Should create elements using DOM, and attach the event using jquery.
Comment #21
kkaefer commentedUse something like
$('<input type="checkbox" />').click(Drupal.checkAll).appendTo(this). You can calculate the parameters in Drupal.checkall becausethisrefers to the input element.Comment #22
kkaefer commentedComment #23
RobRoy commentedThanks for the input guys. Could you take another look at this? I think it's getting closer and it's my first jQuery attempt so I appreciate the input. I've got the checkbox aligned in FF, but still not in IE. Not sure of the cleanest way to do it in CSS.
Comment #24
kkaefer commentedJS side:
varto declare variables. If you don’t do that, they are global.Drupal.checkAllfunction anonymous. That helps to prevent the pollution ofDrupalnamespace.th[@class='checkall'],th.checkallwill do the same and is more bullet proof (If you check forclass='checkall', elements with more than just this class are omitted).Patch in general: I don’t really like this way of checking/unchecking checkboxes. The caveat is that you can’t have more checkboxes in your form (say in another column or below the table) because they would be checked/unchecked as well. It would be cool to save the state of the checkboxes before they are automatically saved so that on uncheck, they are reverted to their previous state.
Comment #25
kkaefer commentedExample:
(This does not address my concerns on the patch in general)
Comment #26
RobRoy commentedOkay, I covered the changes.
This is what I had initially done, but people were worried about CSS pollution. So this patch is pretty much for the specific task of using filter options on the node/comment/users admin pages and being able to check all or clear your selection if you made a mistake a want to select a specific item, for example. For that much needed functionality, this works great IMO.
Comment #27
RobRoy commentedComment #28
lyricnz commentedIt might be nice to look for any element with class=checkall, rather than just "td", which would allow more flexability in where the checkbox was, and not make assumptions about how the form is structured.
Also, perhaps the HTML/checkbox should go *inside* the element, rather than appending to it. For normal forms, I think the last patch would make:
<tr><th class="checkall"></td><b><input type="checkbox"></b><th>Next Heading</th>..rather than
<tr><th class="checkall"><b><input type="checkbox"></b></td><th>Next Heading</th>..which is more accurate.
Comment #29
RobRoy commentedI considered doing all "checkall" class. That's easy enough to do if others agree, maybe we should make the class name a little less common then.
The jQuery expr is good as is me thinks. Here is jQuery doc on appendTo:
appendTo(expr)
Append all of the matched elements to another, specified, set of elements. This operation is, essentially, the reverse of doing a regular $(A).append(B), in that instead of appending B to A, you're appending A to B.
Parameters
* expr (String): A jQuery expression of elements to match.
Comment #30
RobRoy commentedHere's a new patch for HEAD. The only changes are:
- It now finds any 'checkall' class and inserts a checkbox in it (not only a TH tag)
- The checkbox has the form-checkbox class applied to it for better styling
Comment #31
gregglesThis works great for me, but it's worth noting the comment in #16 that this no longer handles the user permissions check all.
Comment #32
RobRoy commentedYeah, I removed that on purpose as it could be dangerous to have a check all for the perms page. So is it looking good? Let's get some higher up's eyes on this!
Comment #33
cutesimaus commentedI cant make it work.
Can someone explain on how I can modify my code to implement this?
I am trying to add the checkAll in my simplenews registration checkboxes
Comment #34
lyricnz commentedAdding a checkall checkbox to an existing form should be simple. Here's an example from the patch itself
The lines starting with '+' are new lines. You can see that you need to add
drupal_add_js('misc/checkbox.js');
to the top of your form-generation code, then add any HTML element with a class of "checkall" to the form itself. The example above puts it as a "heading" on one of the table columns. When the javascript runs, it will insert a checkbox into the element with the checkall class. This checkbox will perform the "checkall" function.
Comment #35
kkaefer commentedComment #36
chx commentedThis was http://drupal.org/node/20641 but now I do not want to mark this duplicate because this is so nice and ready. This is a major usability boost for admins and yet the code is extremely small and nice just as we like it here, thanks lyrincz, kkaefer and RobRoy I have not changed anything just rerolled it to HEAD.
Comment #37
drummThere isn't any indication of what the top checkbox will do. I think it should probably be a text link.
Comment #38
RobRoy commentedGood call, I had similar thoughts but was scared to try and implement it with my limited jQuery skills.
It's working now as proof of concept (the links generation would probably need to be moved to theme_checkall_links() or something), but I need help on to why it won't work using .is() in checkbox.js from someone. Then, I'll clean it up.
Comment #39
drumm"Then, I'll clean it up."
Comment #40
RobRoy commentedI put is as code needs review because I needed someone with some jQuery skills to tell me why that .is() won't work. But, yeah I guess it was a bit suspect.
So can anyone help me out with the .is()? This thing is getting close.
Comment #41
RobRoy commentedHere's an updated patch that throws the links in a theme_checkall() to clean it up a bit. Still need someone to look at that jQuery .is(), then it will be ready for review.
Any other suggestions on this?
Comment #42
m3avrck commentedI have a new patch, that is half the size, coming in a few :-)
Comment #43
m3avrck commentedHere's an updated patch, cleaning up a number of things:
Should be rock solid now, great job everyone!
Comment #44
m3avrck commentedSorry for the misspellings there and not closing my unordered list which this post is for ;-)
Comment #45
m3avrck commentedBetter patch coming soon with some more functionality...
Comment #46
m3avrck commentedOk here's an even better patch than my last, the changes:
All of these changes in a tiny bit more of code, mostly new comments.
Big thanks goes to John Resig for helping with a few trickier parts and giving the A-ok that the JS looks great.
No other changes to the patch, only the JS, all self contained.
Comment #47
Steven commentedI still think a checkbox is preferable to a link. It takes up less space and doesn't suffer from the width jumping. Adding a title attribute should help clarify its function, although its position in the table header is already a big hint.
I made various comments about the code to m3avrck on IRC re code quality.
Comment #48
m3avrck commentedHere's an updated patch after talking with Steven on IRC. Changes:
Still up for debate: check all text verse a checkbox.
I'm in the camp with Steve -- a checkbox with a title attribute is visually pleasing and makes more sense. However, I left the current patch with "check all" text instead. It can be updated to use a checkbox if people want.
Comment #49
m3avrck commentedComment #50
m3avrck commentedUpdated patch with extra line in system.css per chx's request and getting rid of .show() in favor of html.js for Steven.
Comment #51
m3avrck commentedOne last cleanup, get rid of unneeded parameters.
Comment #52
moshe weitzman commentedworks as advertised
Comment #53
m3avrck commentedAny consensus on whether "select all" text should be a checkbox with a title attribute?
I think it should and so should Steven.
But seems Neil and RobRoy think otherwise.
We can always change that later after the patch goes in -- that shouldn't hold it up.
Comment #54
AmrMostafa commentedTested, works. great patch :-).
I like it this way. Being a checkbox would be good also, but I see one advantage to links: it tells what it's gonna do "select all/select none".
Comment #55
sepeck commentedA checkbox on top is fairly consistent with several of the sites I use and many of the GUI interfaces on programs I use as well.
Comment #56
m3avrck commentedHere's an updated patch which adds in *reverse range checking* ... e.g., use range checking to select a bunch in a row, then if you want to uncheck a range within this range, you can do that now :-)
Comment #57
Zen commentedA message from the pro-checkbox cabal:
Checkbox++.
Thanks,
-K
Comment #58
m3avrck commentedOk, here's a new and hopefully final version.
Having a checkbox at the top makes the most sense. It works much better visually, and it really improves the amount of code and "hackish" features needed to introduce text.
So I changed that around. And also made a lot of optimizations, reducing the code, and speeding it up.
Additionally, I've paid super attention to detail -- even the title attribute on the select all box changes depending on the state and forward and reverse and single and subquery range selects all work too.
Very rock solid now.
Big thanks to John Resig (creator jQuery) for helping me in a few parts, giving me some pointers, and really helping to solidify this patch for core.
Comment #59
m3avrck commentedProb needs a review since I changed things around a bit ;-)
Comment #60
m3avrck commentedAdd in t() fix which was accidentally removed.
Comment #61
kkaefer commentedWow, I am amazed how this patch evolved in the last 24 hours! The Shift+Click is really awesome. I tested the patch on Safari and didn't find any problems there.
Comment #62
m3avrck commentedHere's a tiny update which fixes the select-all checkbox title from not updating in certain cases.
Comment #63
webchickWow!!
I speak as a normally crumudgeony person when it comes to throwing JavaScript everywhere that this makes a HUGE improvement. The Shift+Click thing is the best thing since sliced bread. I found the single checkbox kind of weird, but when I hovered over it it did indeed tell me that it selected all checkboxes, so woohoo!
This is RTBC.
Comment #64
RobRoy commentedI actually prefer and was using a checkbox on top, but drumm suggested I try a link so I changed my RTBC patch from #36. The shift+click is definitely nice although the code is a bit bulkier than the other patch.
I noticed two minor things:
1) When you have 50 rows say on the "Posts" page, hitting the Select all box takes about 2 seconds to actually check all the rows on the page. My patch in #36 has no delay. I definitely like the shift+click functionality and highlighting in this patch so I'm not saying the other one is better. But this could maybe use some optimization. Anyone else experiencing this in FF?
2) Since we are always overriding the title="Select all rows in this table" with a translated string passed from PHP, is it necessary to define that in checkbox.js? I would prefer leaving it out if it is redundant.
Nice work m3avrck!
Comment #65
m3avrck commentedRobRoy, great catch on the redundant, hardcoded select all text, I fixed that.
Also, I found an obvious performance bottleneck and fixed that as well. Should be better.
Comment #66
RobRoy commentedJust tested in IE6 and when I click the check all box, nothing checks except the check all box. Then, if I click somewhere else on the page like the filter drop down, then about 5 seconds later all 50 posts get selected. This was because the .change() on line 14 doesn't get triggered in IE6 until changing the focus off that form I think.
- I changed the default title to use the passed in t() string variable instead of being hardcoded as I noticed that wouldn't change until there was a change on the form.
- Changed the .change() to a .click() which also required two new lines to change the select all box's title when all the rows were checked manually.
It works in IE6 fine now, but with 50 rows it still lags pretty hard. Any optimization tips m3avrck?
Comment #67
gregglesWell that was quick. I tested the check_9 and it did feel slow on an older machine of mine with a full page of nodes.
I then tested check_10 on the same machine, same set of nodes, and if I click the box and then move to the dropdown to take an action by the time I get my mouse to the dropdown the JS is done checking all the boxes. That's good enough for me.
There is one slightly odd behavior I noticed - if you shift+select a range and then unselect a few boxes in the middle it toggles the screen from regular to yellow. I believe the desired behavior would be to stay non-yellow, but that seems like a pretty trivial corner case to me.
Comment #68
m3avrck commentedOk updated patch, with code optimization + IE6 fix from RobRoy, swiching from .change() to .click()
Greg, can't reproduce your trivial bug.
Comment #69
RobRoy commentedMuch faster, works in IE6 great. This is solid now. RTBC.
I think greggles is saying, check boxes 3, 4, and 5. Then shift click from 1 to 10 and it will result in 3-5 being deselected while 1-2, and 6-10 are selected. I think this is fine.
Comment #70
dries commentedSweet! Works fine in Firefox. :-)
Comment #71
m3avrck commentedAh yes. Ok I did some research. Google Gmail doesn't exhibit weird behavior like that.
So I fixed the patch so we don't show that anymore. Should be fixed now Greg :-)
It was a simple 1 line fix anywho ;-)
Comment #72
RobRoy commentedA shift+click bug:
1. Hit select all
2. Uncheck #8
3. Then check #8 so it is selected again (now all boxes are yellow again)
4. THEN shift+click #8 like you were preparing to deselect 8 through 12 or something. But wait! When you hit shift+click on #8, #8 through the end get deselected.
Comment #73
m3avrck commentedConfirmed RobRoy's bug, turns out there was a small missing statement in the if, fixed now.
Comment #74
m3avrck commentedHere's an updated patch which clarifies which TR parent to use, this fixes breakage in some themes. Was another very minor change.
Comment #75
Zen commentedAs mentioned on IRC, having three different t() arrays in three different places is wasteful. Please stick it all in a helper function..
Perhaps something like the following will simplify things a bit:
and in the theme function:
The function name could probably use some work :)
Setting to code needs work.
Thanks,
-K
P.S I would personally have the shift-select functionality available throughout the site by default, but that might just be me.
Comment #76
kkaefer commentedI made some alterations to the JavaScript. It is now possible to have more than one table on a page and even in one form.
Other changes:
Comment #77
m3avrck commentedHere's an updated patch, changes:
var checkboxes = $('input:checkbox', table).not(checkAll).click(function(e) {has had the .not() removed, jQuery wasn't parsing the checkAll variable correctly, updated the $() to be more specific so we didn't need it, will be faster too nowComment #78
m3avrck commentedOops, patch was missing the JS file.
Comment #79
m3avrck commentedFound out this code didn't work for themes that had tabled based layouts. I updated the $(document).ready() selector be specific and find tables within forms. Working well now.
As of now *all* issues within this thread have been fixed and code has been rewritten 3 times now, it's rock solid.
If there are *no* new issues, I'd say this is RTBC.
Comment #80
RobRoy commentedMinor changes:
- Changed the calls to theme('table_select_header_cell') instead of theme_table_select_header_cell()
- Fixed some whitespace
- Added some periods to code comments
Comment #81
eaton commentedDude. This is really, really slick.
I know it's a lifesaver on sites that've been comment-spammed and didn't have any useful blocking software in place.
Just tried it on a fresh HEAD on a number of pages, with a few combinations of nodes and comments and what-not. Works like a charm. A big +1.
Comment #82
lyricnz commentedYeah, looks good for me, too.
Comment #83
jjeff commentedTested. This is rocking... RTBC
Comment #84
Frando commentedWorks exactly as described. Great improvement!
Comment #85
m3avrck commentedUpdate the scope for a couple variables to be unambiguous.
Comment #86
Steven commentedFixed the bug where unselecting a range immediately after selecting it would leave the checkbox at the end untouched.
I also added some improved styling for Garland rather than relying on the generic yellow. Selected table rows now invert in Garland, just like in typical desktop GUIs.
http://acko.net/dumpx/Afbeelding%204.png
Committed to HEAD.
Comment #87
webchickOh, awesome!!! :D Thanks, Steven!! :D
Comment #88
(not verified) commentedComment #89
liquidcms commentedvery cool.. but sadly i guess only for Drupal 5 (which i think is still a good 6 months away from being usable for most of my applications)...
but, possibly this could be done for 4.7?
- add function to theme.inc
- include jsquery libary somewhere
- add single line to node.module (or whichever)
- include tableselect.js file
any thoughts on if i am wasting my time heading down this path?
Comment #90
liquidcms commentedoh well, guess not... didn't really expect it would be that simple.
Comment #91
afagioliI've done something about the chekall issue for the previous Dpupal release, by some javascript code
Step 1.
Add the javascript function to your theme page.tpl.php this function
function DoToAll(obj_input){ bol_is_checked = (obj_input.checked)?true:false for (x=0;xbe sure is inside the head of the html document
Step 2. Add the following line at node.module:1151 core
module row 1151
$form['options']['CheckAll']
= array('#type' => 'markup', '#value' => 'Check All type="checkbox" onclick="DoToAll(this)">
');
If interested, you'll find more info at www.fagioli.biz
Cheerio!
Comment #92
summit commentedHi,
I am interested in this "select all" checkbox in relation to the taxonomy batch operation functionality (drupal 5).
Did somebody already use this? Is this in core now, if so, is there a setting to get this roking on admin/content/taxonomy pages ?
Greetings,
Martijn
Comment #93
kkaefer commentedThis is not a forum post. It is an issue that was specifically opened for getting the described functionality into core. It is considered archived now. Please do not reply to this issue. If you have questions, use the Drupal.org forums.
Comment #94
Stan.Ezersky commentedIt works for me
[Edited by kiamlaluno to show the code indented]
Comment #95
gfxguru commentedCan we get the checkall feature integrated into the latest D6 build? Does anyone believe this should be a core feature? Trying to distinguish which patch to use and which one works is troublesome.