Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
user system
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
14 Nov 2005 at 03:55 UTC
Updated:
27 Jan 2010 at 04:46 UTC
Jump to comment: Most recent file
Comments
Comment #1
crunchywelch commentedhere is one with the document root set to drupal root, if it matters.
Comment #2
crunchywelch commentedbit of code cleanup
Comment #3
crunchywelch commentedkilles says dont send null values to db_query() ;)
Comment #4
crunchywelch commentedand some breadcrumbs to display all roles if in a filtered view
Comment #5
crunchywelch commentedchanged the url style to match taxonomy.module, and use the same regex routine as well...
Comment #6
crunchywelch commentedand now, without drupal.css changes....
Comment #7
chx commentedIN is faster than OR and makes shorter code, too. I love this one.
Comment #8
crunchywelch commentedMuchas gracias for the reviews, looks great and ready to commit
Comment #9
chx commentedWhen this gets in, one of our JS guys can do their freaky JS magic on this screen to make columns dynamically switchable... it does not even need communication w/ server, only the POST action URL needs to change according to visible (and posted) columns.
Comment #10
Crell commentedThe JS side was already discussed in this thread: http://drupal.org/node/30843
The patch as is is nice, but only somewhat useful. The UI to show an arbitrary subset of roles rather than just one role is rather crucial for it being fully realized. It can go in as is, I suppose, but the filtering interface needs to happen soon then, be it server-side or client-side or both.
Of course, if it's all client-side javascript then this patch is really unneeded, since just hiding or showing different roles client-side won't change the data that's sent back to the server nor what the server has to do with it.
So I guess it seems like it's solving the wrong problem. :-)
Comment #11
chx commentedNo it's not. If you display: none something it won't get into $_POST so we need to change the save code so that it saves only relevant roles. Also, we always need a non-JS fallback.
Comment #12
Thox commentedJust to note on chx's comment: form controls hidden with CSS do get submitted and are available in $_POST. Only controls that are disabled or taken out of the DOM (using javascript) will not be submitted.
On the other hand, a non-JS solution to the roles page is worth having.
Comment #13
m3avrck commentedThis patch does work as intended and is pretty nifty, JS could definetly do a lot with this. One idea for non-JS, would be to have a multi-select with all the roles at the top of the page, that has all roles or certain roles selected by default. The user could then ctrl+select the roles they want to see. Seems this patch paves the way for that to happen, perhaps it should do that as well? +1 on the idea, but I think the patch needds a tiny bit of work (multi-select or similar) to really make it useful.
Comment #14
Steven commented-1 for code style (if spacing) and unhappy SQL usage. The IN(%s) should really be avoided. In this case it's safe I think because the $rids is validated by preg, but the proper way to do it is to construct an appropriate IN(%d,%d%d) statement and pass the values individually to db_query(). We don't want to set bad examples.
Comment #15
dries commentedComment #16
Jaza commented+1 to this feature. Let's get this in first, a UI can come later.
Moving to 6.x-dev queue.
Comment #17
catchComment #18
Jaza commentedWe now have filtering by a single rid, and I'd say that's enough.
Closing.