Issue status update for http://drupal.org/node/20457 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: jhriggs -Status: active +Status: patch Attachment: http://drupal.org/files/issues/roles_div.patch (930 bytes) Ugh...and the patch jhriggs Previous comments: ------------------------------------------------------------------------ April 13, 2005 - 13:34 : Morbus Iff Attachment: http://drupal.org/files/issues/_patch_usermodulebr.patch (848 bytes) On admin/user, the user's roles are separated both by a comma and a [br /]. The comma is quite enough - the [br /] merely makes the page look retarded ("fat" rows) on screens that have plenty of width to accomodate the entire table without the BR. On smaller width'd browsers, the "fat" rows will return, but naturally wrapped, not forced with inline HTML. Likewise, [BR] and the comma are both delimiters: only one is needed (if we use [BR], the newline becomes the visual delimiter; if we use comma, then we don't need the newlines). The attached patch removes the [BR]. ------------------------------------------------------------------------ April 13, 2005 - 13:45 : Dries Committed to HEAD. ------------------------------------------------------------------------ April 13, 2005 - 13:52 : jhriggs I'm not sure how I feel about this. Removing the break is OK, I suppose, but removing the comma would not be. You could not have breaks alone, because roles can have spaces in them, and they may not break properly. So, we need to leave the commas in. Personally, I think it is nicer to see what we have now: authenticated user, administrator, web site user rather than: authenticated user, administrator, web site user We probably shouldn't have the hardcoded break, but maybe they should be in a div. That way a theme can display them inline if desired. ------------------------------------------------------------------------ April 13, 2005 - 13:53 : jhriggs Cross-posted with Dries there. Setting back to active, because I don't think this is the best fix. ------------------------------------------------------------------------ April 13, 2005 - 14:20 : jhriggs Here's a patch that wraps each role in a div: <div class="role">authenticated user,</div> <div class="role">foo bar,</div> <div class="role">baz blah</div> This way they still appear on separate lines unless the theme decides to make div.class display inline.

Issue status update for http://drupal.org/node/20457 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: Bèr Kessels Status: patch +1 from me. However, were we not going to make a generlal wrapper function? Should we then not wait for that function, instead of seeding HTML all over the place? Bèr Kessels Previous comments: ------------------------------------------------------------------------ April 13, 2005 - 20:34 : Morbus Iff Attachment: http://drupal.org/files/issues/_patch_usermodulebr.patch (848 bytes) On admin/user, the user's roles are separated both by a comma and a [br /]. The comma is quite enough - the [br /] merely makes the page look retarded ("fat" rows) on screens that have plenty of width to accomodate the entire table without the BR. On smaller width'd browsers, the "fat" rows will return, but naturally wrapped, not forced with inline HTML. Likewise, [BR] and the comma are both delimiters: only one is needed (if we use [BR], the newline becomes the visual delimiter; if we use comma, then we don't need the newlines). The attached patch removes the [BR]. ------------------------------------------------------------------------ April 13, 2005 - 20:45 : Dries Committed to HEAD. ------------------------------------------------------------------------ April 13, 2005 - 20:52 : jhriggs I'm not sure how I feel about this. Removing the break is OK, I suppose, but removing the comma would not be. You could not have breaks alone, because roles can have spaces in them, and they may not break properly. So, we need to leave the commas in. Personally, I think it is nicer to see what we have now: authenticated user, administrator, web site user rather than: authenticated user, administrator, web site user We probably shouldn't have the hardcoded break, but maybe they should be in a div. That way a theme can display them inline if desired. ------------------------------------------------------------------------ April 13, 2005 - 20:53 : jhriggs Cross-posted with Dries there. Setting back to active, because I don't think this is the best fix. ------------------------------------------------------------------------ April 13, 2005 - 21:20 : jhriggs Here's a patch that wraps each role in a div: <div class="role">authenticated user,</div> <div class="role">foo bar,</div> <div class="role">baz blah</div> This way they still appear on separate lines unless the theme decides to make div.class display inline. ------------------------------------------------------------------------ April 13, 2005 - 21:20 : jhriggs Attachment: http://drupal.org/files/issues/roles_div.patch (930 bytes) Ugh...and the patch ------------------------------------------------------------------------ April 13, 2005 - 22:58 : factoryjoe Attachment: http://drupal.org/files/issues/roles_span.patch (944 bytes) I would prefer that these be spans, since they're the least semantically offensive. If you want to have breaks, you can do so in your theme (span.user-role{display:block;}), otherwise they sit snuggly in a single line.

Issue status update for http://drupal.org/node/20457 Post a follow up: http://drupal.org/project/comments/add/20457 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: leafish_paul Status: patch Just to stir things up a bit, you could also say its a list, and define HTML and CSS to display it as desired, whether that be inline or as a block list... Anyways, I was just checking this patch against CVS - does it still apply? I no longer see a column for roles in the table at all. It seems user_admin_account() function no longer SELECTs or displays it? <?php function user_admin_account() {   $header = array(     array('data' => t('Username'), 'field' => 'u.name'),     array('data' => t('Status'), 'field' => 'u.status'),     array('data' => t('Member for'), 'field' => 'u.created', 'sort' => 'desc'),     array('data' => t('Last access'), 'field' => 'u.access'),     t('Operations')   );   $sql = 'SELECT u.uid, u.name, u.status, u.created, u.access FROM {users} u WHERE uid != 0'; ?> leafish_paul Previous comments: ------------------------------------------------------------------------ Wed, 13 Apr 2005 19:34:39 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_patch_usermodulebr.patch (848 bytes) On admin/user, the user's roles are separated both by a comma and a [br /]. The comma is quite enough - the [br /] merely makes the page look retarded ("fat" rows) on screens that have plenty of width to accomodate the entire table without the BR. On smaller width'd browsers, the "fat" rows will return, but naturally wrapped, not forced with inline HTML. Likewise, [BR] and the comma are both delimiters: only one is needed (if we use [BR], the newline becomes the visual delimiter; if we use comma, then we don't need the newlines). The attached patch removes the [BR]. ------------------------------------------------------------------------ Wed, 13 Apr 2005 19:45:03 +0000 : Dries Committed to HEAD. ------------------------------------------------------------------------ Wed, 13 Apr 2005 19:52:22 +0000 : jhriggs I'm not sure how I feel about this. Removing the break is OK, I suppose, but removing the comma would not be. You could not have breaks alone, because roles can have spaces in them, and they may not break properly. So, we need to leave the commas in. Personally, I think it is nicer to see what we have now: authenticated user, administrator, web site user rather than: authenticated user, administrator, web site user We probably shouldn't have the hardcoded break, but maybe they should be in a div. That way a theme can display them inline if desired. ------------------------------------------------------------------------ Wed, 13 Apr 2005 19:53:09 +0000 : jhriggs Cross-posted with Dries there. Setting back to active, because I don't think this is the best fix. ------------------------------------------------------------------------ Wed, 13 Apr 2005 20:20:10 +0000 : jhriggs Here's a patch that wraps each role in a div: <div class="role">authenticated user,</div> <div class="role">foo bar,</div> <div class="role">baz blah</div> This way they still appear on separate lines unless the theme decides to make div.class display inline. ------------------------------------------------------------------------ Wed, 13 Apr 2005 20:20:45 +0000 : jhriggs Attachment: http://drupal.org/files/issues/roles_div.patch (930 bytes) Ugh...and the patch ------------------------------------------------------------------------ Wed, 13 Apr 2005 21:58:41 +0000 : factoryjoe Attachment: http://drupal.org/files/issues/roles_span.patch (944 bytes) I would prefer that these be spans, since they're the least semantically offensive. If you want to have breaks, you can do so in your theme (span.user-role{display:block;}), otherwise they sit snuggly in a single line. ------------------------------------------------------------------------ Thu, 14 Apr 2005 09:55:09 +0000 : Bèr Kessels +1 from me. However, were we not going to make a generlal wrapper function? Should we then not wait for that function, instead of seeding HTML all over the place? ------------------------------------------------------------------------ Thu, 14 Apr 2005 17:59:58 +0000 : factoryjoe What's interesting about your point, Ber, is that in this case, we really don't need a wrapper per se... instead, if you just threw a class on the containing TD, you could wrap the roles in spans and then style them that way: <td class="roles">   <span>administrator,</span>   <span>authenticated user</span> </td>However, the broader question here is what should be wrapped by Drupal (and at what level of granularity -- i.e. wrap each role as in this patch or wrap the container with a class?). Furthermore, does it make sense to make this themeable or not? At some point we're going to need to hash out how to make decisions about this kind of thing, since the BRs that were removed were not semantic and were purely presentational. I would strongly suggest erring on the side of semantic defaults that themers can manipulate to their will; the question though, is, what consistent parts of Drupal should be themeable without overcomplicating things?