Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The controls for setting field privacy currently look like:
Instead, they could appear on a dedicated tab in the user profile so that they are centralized in a single place as in the below comp. Note, the default functionality would be "Only to me" and "Everybody" rather than the additional options in the comp, which are site-specific.
Comment | File | Size | Author |
---|---|---|---|
#17 | profile-privacy-ui-1404590-17.patch | 5.77 KB | cpliakas |
#15 | profile-privacy-ui-1404590-14.patch | 8.18 KB | cpliakas |
#9 | alignment-1404590.jpg | 14.94 KB | cpliakas |
#3 | profile_privacy-ui_improvements-1365258.patch | 5 KB | snufkin |
20120113-j3rkxr4849m7tm62exfbdsdqtj.jpg | 44.66 KB | ezra-g |
Comments
Comment #1
jsibley CreditAttribution: jsibley commentedLooks like a great idea. Looking forward to it.
Comment #2
cpliakas CreditAttribution: cpliakas commented100% in favor of this UI change.
Comment #3
snufkin CreditAttribution: snufkin commentedSo far I have the form, the theme function to arrange the radios into a table. I moved the theme function and form to a separate file. Submit callback still needs to be done.
Comment #4
izmeez CreditAttribution: izmeez commented+1 for this idea. Just a couple of questions:
Does "Everybody" mean "all site users"?
Is it also necessary to add a column for "public"?
Comment #5
snufkin CreditAttribution: snufkin commentedWell currently the columns are the audiences defined using the profile privacy module, if you add more then more columns will appear (at least thats the aim).
Comment #6
cpliakas CreditAttribution: cpliakas commentedizmeez,
Maybe, but not necessarily. As discussed in the thread at #299427: Consider changing word "publicly" to distinct registered users and nonregistered users, that depends on permissions. Do anonymous users have the ability to view user profiles? Only a subset of roles? Is there a third party module that is rendering profile information outside of the core Profile page so that the core permissions don't apply? There are a lot of variables in play, but usually this will mean everyone who has permissions to access user profiles.
Thanks,
Chris
EDIT:
Although the image says "Everybody", the text is actually determined by the hook_profile_privacy_access_info() implementations, so the actual text will be "Public" unless it is determined that there is a better alternative. Of course, you can override this string through the normal translation methods.
Comment #7
cpliakas CreditAttribution: cpliakas commentedsnufkin,
Great start! UI looks really good. I know you are still working on this, but there are a couple of other things in addition to the items mentioned in #3 that I want to capture in this thread so we can compile a checklist to put in the issue summary.
Well done,
Chris
Comment #8
snufkin CreditAttribution: snufkin commentedWhat is the issue with the radio buttons?
With regards to the tabs I hope to get some suggestions, I tried to copy the way user module implements the tabs, as in using %user_category and the map element in the menu item, but that resulted in getting a 404 for my page callback (even though the tab showed up on the other user/UID/edit pages).
Comment #9
cpliakas CreditAttribution: cpliakas commentedsnufkin,
Re: the radios, it's not a big deal but the alignment is a bit off. See the image below. Again, it's a matter of pixels and it doesn't really bother me, but if you have a quick fix it would be a good addition.
D6 menus suck :-). I'll take a hack at it to see if we can come up with something that works.
Thanks,
Chris
Comment #10
snufkin CreditAttribution: snufkin commentedhmm I have them in a straight line on chrome + OSX (just checked with skitch too): https://skitch.com/thesnufkin/g3eh2/edit-commons. I'll check it out once the submit callback works.
Comment #11
cpliakas CreditAttribution: cpliakas commentedInteresting. I am on Chrome + OSX as well. I am on Commons 2.3, so that could also be the issue. I will trust your testing on this.
Comment #12
cpliakas CreditAttribution: cpliakas commentedThe menu issue was a bit tricky to track down, and it might be even trickier to fix. Listed below is an explanation of the issue and a couple of ways we could attack it.
The Tabs Issue For The tl;dr Crowd
When the tabs are generated, the
%map
for the load callback is constructed from the current page instead of using each item's path. If you are visiting the privacy page added in the patch, then the load callback will be passed a%map
ofarray('user', $uid, 'edit', 'privacy')
for ALL menu items. Therefore none of the tabs will be displayed because the user_category_load() function won't return an object due to "privacy" not being a profile category.Detailed Tabs Issue
All of the user profile category pages have menu items such as
user/%user_category/edit/My Category
, where the%user_category
is a wildcard loader argument that takes the special load arguments ofarray('%map', '%index')
. The%map
is essentially an array containing the path parts that are used by the user_category_load() function to load the user object after checking if any profile category in the path exists. It needs the map so it can use the category ID at the end of the path (My Category in the example above) in addition to the user ID passed through the position of the%user_category
wildcard argument loader. This is pretty standard menu item stuff.Now it gets interesting. When you visit a tab on the user/*/edit pages, the menu system finds all of the other tabs related to the one you are viewing and performs access checks on each item. If you follow the tab generation from theme_menu_local_tasks(), you eventually get to the _menu_translate() function which is where our problem begins. The second thing that function tries to do is load the objects specified by the wildcard argument loaders via the _menu_load_objects() function. The
$map
variable is the%map
passed to the load callbacks, however it is generated from the current path and NOT each tab's path. Therefore all of the tabs use the same map based on the page you are visiting. If you are on a page where the last path part is a valid category, then all of the other tabs will display becauseuser_category_load()
will always return an object. However if you are on a the privacy page added in the patch, then none of the tabs will display because_menu_load_objects()
will returnFALSE
due to "privacy" not being a valid category.Possible Solutions
Nothing easy jumps out at me. Either we override all of the menu items containing the
%user_category
wildcard argument loader and provide our own that returns an object when the "privacy" page is being viewed, or we fake the "privacy" page being a profile category and simply hide it from the forms where you can add fields to a category page. Are there any other ways we could tackle this?Comment #13
cpliakas CreditAttribution: cpliakas commentedLooks like a core issue has been filed that attempts to solve the issue mentioned above at #1137052: Argument map of parent router item is used when checking access for local tasks..
Comment #14
snufkin CreditAttribution: snufkin commentedThe code is now in 1404590-profile-privacy-ui branch, ready to be reviewed. I also removed the form elements from the users profile edit page and all relevant submit and theme calls.
Comment #15
cpliakas CreditAttribution: cpliakas commentedThanks! Attaching patch to the thread.
Comment #16
cpliakas CreditAttribution: cpliakas commentedMarking as needs work because it seems that a few of the issues merged in from the 6.x-2.x branch are not reflected in the patch. We will have to git cherry-pick the following commits back into the branch so there aren't any regressions when these changed are merged back in.
Comment #17
cpliakas CreditAttribution: cpliakas commentedThe changes mentioned above have been cherry-picked ontl the ui branch, so it is now in-sync with the latest 6.x-2.x branch. Changing back to "needs review".
Comment #18
cpliakas CreditAttribution: cpliakas commentedComment #19
snufkin CreditAttribution: snufkin commentedAdded the new UI to edit the visibility overrides as well: https://img.skitch.com/20120119-giicaxrj7f7m1wqf1t86a4npg.jpg
Comment #20
lsolesen CreditAttribution: lsolesen commented