[development] [User experience feature] Access Control enhancement - add role name to link hover text

[inactivist]

Issue status update for 
http://drupal.org/node/78773
Post a follow up: 
http://drupal.org/project/comments/add/78773

 Project:      User experience
 Version:      <none>
 Component:    usability
 Category:     feature requests
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  inactivist
 Updated by:   inactivist
 Status:       patch (code needs review)
 Attachment:   http://drupal.org/files/issues/user.module.patch_1.txt (1.55 KB)

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.)




inactivist



Previous comments:
------------------------------------------------------------------------

Tue, 15 Aug 2006 16:10:34 +0000 : inactivist

Attachment: http://drupal.org/files/issues/drupal-access-control-with-new-hover.patch.txt (1.67 KB)

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 [1].


/*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./


[1] http://www.twogrunts.com/node/3




------------------------------------------------------------------------

Tue, 15 Aug 2006 16:37:17 +0000 : greggles

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.




------------------------------------------------------------------------

Tue, 15 Aug 2006 18:00:58 +0000 : inactivist

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




------------------------------------------------------------------------

Tue, 15 Aug 2006 19:02:58 +0000 : drumm

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.




------------------------------------------------------------------------

Tue, 15 Aug 2006 19:15:38 +0000 : drumm

Related issue: http://drupal.org/node/78808. I think both changes are
okay (if code review checks out of course).




------------------------------------------------------------------------

Tue, 15 Aug 2006 19:43:41 +0000 : inactivist

"#3 submitted by drumm on August 15, 2006 - 11:02
The comment should be a simple description/explanation of what is
happening. Save the personal pronouns or proposals for here.

"
Will fix.  


"The use of t() needs some work. See
http://api.drupal.org/api/HEAD/function/t

"
Would you mind elaborating?


Thanks.




------------------------------------------------------------------------

Tue, 15 Aug 2006 20:08:26 +0000 : inactivist

Attachment: http://drupal.org/files/issues/drupal-access-control-with-new-hover.patch_0.txt (1.55 KB)

Ok, here's my latest patch against 4.7 cvs.


- Cleaned up comments
- Proper use of t() (please review and confirm)