| Project: | Drupal core |
| Version: | x.y.z |
| Component: | user system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (duplicate) |
Issue Summary
One of the things that frustrates me when using drupal is the access control feature. I think the basic idea is fine; it's just that the UI for the access control panel (a grid of checkboxes) can be cumbersome to work with when you have multiple non-standard roles defined.
My straightforward solution is to prepend the text "allow [role name] to " the tooltip text for the checkbox.
For background and details, see my write-up on twogrunts.com.
Note: this is my first attempt at submitting a patch to anything in the drupal project, core or otherwise. I'm a newbie (to drupal and PHP), so please, let me know if I'm taking the wrong approach here, or if there's an easier way to do this. In particular, I wasn't sure what project and component to assign this to.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal-access-control-with-new-hover.patch.txt | 1.67 KB | Ignored: Check issue status. | None | None |
Comments
#1
This is very similar to the goal of http://drupal.org/node/28301 though 28301 which should have been implemented in http://drupal.org/node/44379 but it seems to have fallen out of that issue.
Basically, there's lots of people who agree this is a good idea, though moshe's comment#13 on 28301 seems to indicate that it should be simpler than the patch by inactivist.
#2
Awesome! I had a feeling this wasn't new. And I'm glad to see that there's a cleaner way to implement this.
So, this raises one question (for me, anyway) - why do useful patches like this not make it into the mainline releases? Is there something that we need to do in order to ensure that something like this makes it into the next 4.7.x release?
-Mike
#3
The comment should be a simple description/explanation of what is happening. Save the personal pronouns or proposals for here.
The use of t() needs some work. See http://api.drupal.org/api/HEAD/function/t.
#4
Related issue: http://drupal.org/node/78808. I think both changes are okay (if code review checks out of course).
#5
Will fix.
Would you mind elaborating?
Thanks.
#6
Ok, here's my latest patch against 4.7 cvs.
- Cleaned up comments
- Proper use of t() (please review and confirm)
#7
This is the same as the previous patch, except uses '%role : %perm' as the format string, which is more appropriate (for example, the adsense module has a permission of "hide adsense" which, in the previous patch, would result in "allow %role to hide adsense" - and is totally incorrect.)
#8
Patch diff'd against DRUPAL-4-7 tag. (I suspect I was not using CVS correctly on previous diffs. Still trying to get a clue. Let me know if I'm unsuccessful.)
#9
Moving to Drupal project. Should use
user_roles()to load the available roles.#10
Good catch. Still learning the APIs. Thanks.
Here's the updated patch against 4.7.
Should I be running the patches against the latest CVS (HEAD) ?
#11
I think Neil's approach is the better one here -- it much more intuitive. Often times people don't see these tooltips (eg. I have them disabled) or it takes 2 seconds for them to show up.
Tempted to mark this duplicate of http://drupal.org/node/78808.
#12
I prefer Neil's approach too.
#13
Theres no reason why we can't do both.