Opened 12 years ago
Closed 8 years ago
#10741 closed enhancement (fixed)
[Patch] Provide indicator for verification status of email addresses on the Manage User Accounts page
Reported by: | Ryan J Ollos | Owned by: | Steffen Hoffmann |
---|---|---|---|
Priority: | normal | Component: | AccountManagerPlugin |
Severity: | minor | Keywords: | web-UI CSS |
Cc: | Trac Release: |
Description
The verification status of an email address for an individual user can be seen by navigating to the Account Details page. However, on installations with many users, it would be nice to be able to see which accounts haven't been verified while viewing the Manage User Accounts page.
The initial idea I have for this is to the use the light grey color that is used for missing permissions (see t:#10752) on the Manage Permissions and Groups page. How does that sound? Any better ideas?
Attachments (9)
Change History (37)
comment:1 Changed 12 years ago by
Keywords: | web-UI CSS added |
---|---|
Severity: | normal → minor |
comment:2 Changed 12 years ago by
Thanks for the quick feedback. The patch in t:#10742 just applied the missing
class to inactive permissions. This is the same class used for missing tickets, wiki pages, etc. So the first thought I had was to do the same for email addresses that haven't been validated and I'll work up a patch for this.
comment:3 Changed 12 years ago by
The icons for locked accounts sound very useful, btw. Looking forward to seeing that.
Changed 12 years ago by
Attachment: | Accounts.png added |
---|
comment:5 Changed 12 years ago by
It looks like fetch_user_data
could be called in _do_acct_details
to eliminate some redundant code. I'll investigate that in another ticket later on.
Changed 12 years ago by
Attachment: | th10741-r12500-1.patch added |
---|
Patch against r12500 of the trunk.
comment:6 Changed 12 years ago by
Summary: | Provide indicator for verification status of email addresses on the Manage User Accounts page → [Patch] Provide indicator for verification status of email addresses on the Manage User Accounts page |
---|
comment:7 Changed 12 years ago by
This looks like a good start, but because users with unverified account are treated similar to users in an anonymous session, I'll alter this into graying text of the whole row.
And the upcoming account approval/ban feature will use the same representation, probably with different tool-tip (title HTML tag). And different icon, that is what I've been working on before diving more into substantial enhancements with AcctMgr lately.
comment:8 follow-up: 9 Changed 12 years ago by
I guess that graying out the whole row is okay. I rather liked the idea of just having an explicit indicator on the email, but I'm not too wed to either approach.
comment:9 Changed 12 years ago by
Replying to rjollos:
I guess that graying out the whole row is okay. I rather liked the idea of just having an explicit indicator on the email, but I'm not too wed to either approach.
Seen that and I like the idea too, but OTOH this might suggest an almost usable account, while quite the contrary is true: Until email is verify, there's hardly a difference to an unauthenticated session, given all elevated permissions are wiped for each request.
Changed 12 years ago by
Attachment: | EntireRowGrey.png added |
---|
Changed 12 years ago by
Attachment: | th10741-r12501-1.patch added |
---|
comment:10 follow-up: 12 Changed 12 years ago by
I've implemented the suggestion of graying out the entire row in th10741-r12501-1.patch. The patch also implements #10743, and adds a title to the link for reviewing user attributes. Now that I've seen the change in action, I do prefer graying out the entire row rather than just the email address.
Changed 12 years ago by
Attachment: | MoreInfo-Expanded.png added |
---|
Changed 12 years ago by
Attachment: | MoreInfo-Unexpanded.png added |
---|
comment:11 Changed 12 years ago by
I'm thinking that eventually we should replace the link on the account name with a More details icon in the far right column.
Alternatively, we could drop the Review user account details page and show those details on the Manage user accounts page when expanding a More info section. Here is an example from a site that I use:
I'm partial to the latter idea at the moment, though it looks like it will only work when JavaScript is enabled, so we'd probably want to leave the Review user account details in place as a fallback when JavaScript is not enabled.
comment:12 Changed 12 years ago by
Replying to rjollos: ...
Now that I've seen the change in action, I do prefer graying out the entire row rather than just the email address.
I'm glad that we agree on this, but I've been unable to reproduce the gray-out be adding 'missing' class to 'tr' HTML tag. While I succeeded after adding a custom CSS rule, it still doesn't apply to the link labels, so could you investigate, where this CSS is coming from in your setup, please?
comment:13 follow-up: 14 Changed 12 years ago by
Yeah, I'll investigate. I'm not very proficient with CSS, so I wouldn't be surprised if the rule could be coded better. What version(s) of Trac did you test with?
Changed 12 years ago by
Attachment: | Chrome23-CSS.png added |
---|
comment:14 Changed 12 years ago by
Replying to rjollos:
Yeah, I'll investigate. I'm not very proficient with CSS, so I wouldn't be surprised if the rule could be coded better. What version(s) of Trac did you test with?
I've been testing with Trac-1.0 by now, but I'm fine with a few extra CSS rules in acct_mgr.css, and it's actually working fine now. So don't dig for it too long - just make sure to test, if my final version is working for you as well.
comment:15 follow-up: 16 Changed 12 years ago by
Am I reading your comment correct then that it works with the CSS added in admin.css
, and just didn't have that included in your initial testing?
I was about to post the following:
I've tested with several browsers on Debian 6 with Trac 1.1.1dev-r11483
: Chrome 23, FF 17, Epiphany 3.4.2.
Here is what I see in Chrome 23:
comment:16 Changed 12 years ago by
Replying to rjollos:
Am I reading your comment correct then that it works with the CSS added in
admin.css
, and just didn't have that included in your initial testing?
Sorry, I had overseen that, because I just extracted parts of the proposed changes at a time, so nevermind.
comment:17 Changed 12 years ago by
I like the idea about moving the link to a dedicated column and label.
But the further it goes into implementation detail the more I get discouraged by the growing complexity. As witnessed with the email link issue (#10743) AcctMgr has already quite complex code structures, and I'd like to keep it at a reasonable level for now.
I don't say 'Never', but these changes won't happen in the near future. For now it works well, and the only thing that could force that change would be the introduction of some dedicated user page, like we have it at t-h.o, but this is not yet a generally agreed pattern.
comment:18 Changed 12 years ago by
That's fine. I'll probably be doing custom templates for bloodhound anyway, and can just keep the work over there if you don't wish to include it in AccountManager core.
comment:19 Changed 12 years ago by
(In [12505]) AccountManagerPlugin: Some enhancements regarding approval/ban feature, refs #843, #8595 and #10741.
Adding the registration approval configuration option to config admin panel. Taking care for marking all potential message parts for translation as well. A recent suggestion by Ryan J Ollos is merged in here too, so that all kinds of restricted accounts are clearly visible in user listings now.
comment:20 follow-up: 23 Changed 12 years ago by
I've tested with Trac 1.1.1dev
and everything is working well. I like that the configuration panel is starting to take shape now, with multiple options per fieldset. Before, it seemed overly boxed, but along with the change in [12506] in which two fieldsets were combined, it is starting to look nice, and logically organized.
I'll have to remember to take care in the future for translations. So far I have trained myself to remember to do this in Python code, but frequently tend to forget when working in the templates.
comment:21 Changed 12 years ago by
Here is a potential issue with [12506]. The system-message
wasn't moved when the Password Reset fieldset was moved:
Changed 12 years ago by
Attachment: | SystemMessage.png added |
---|
comment:22 follow-up: 24 Changed 12 years ago by
Although the more I look at this, I think maybe the system-message
should be displayed in the location shown in comment:21. On first look, it just appears slightly out of place, particularly since add_notice
is usually used to position similar messages at the top of the page.
comment:23 Changed 12 years ago by
Replying to rjollos:
I've tested with Trac
1.1.1dev
and everything is working well. I like that the configuration panel is starting to take shape now, with multiple options per fieldset. Before, it seemed overly boxed, but along with the change in [12506] in which two fieldsets were combined, it is starting to look nice, and logically organized.
Ah well, you're welcome. It just occurred to me, that there was a more logical order, and binding related options would make it look even clearer, and I took the chance.
comment:24 follow-up: 25 Changed 12 years ago by
Replying to rjollos:
Although the more I look at this, I think maybe the
system-message
should be displayed in the location shown in comment:21. On first look, it just appears slightly out of place, particularly sinceadd_notice
is usually used to position similar messages at the top of the page.
I agree, that this is an odd place according to all standards. No wonder, I didn't take care for it. I would rather have it above the new Refresh/Reset, or even better: inside the section, just to make clear what you did right now. Or the standard location on-top of the page, sure. But it looks a bit too far away.
Changed 12 years ago by
Attachment: | 20130104_admin_acctcfg-SystemMessage.png added |
---|
suggestion: restart-confirmation replaced right above dedicated button
comment:25 follow-up: 26 Changed 12 years ago by
Replying to hasienda:
Or the standard location on-top of the page, sure. But it looks a bit too far away.
Yeah, I agree, it would be too far away at the top of the page. I like the placement you propose in 20130104_admin_acctcfg-SystemMessage.png.
In fact, I've been considering a patch to the Trac core for the plugin admin page that is patterned after your inline notices. After enabling or disabling a component, there is a notice at the top of the page, but it is usually out of site since the redirect contains a fragment that lands the user on the plugin for which the action was invoked.
comment:26 Changed 12 years ago by
Replying to rjollos:
Replying to hasienda:
Or the standard location on-top of the page, sure. But it looks a bit too far away.
Yeah, I agree, it would be too far away at the top of the page. I like the placement you propose in 20130104_admin_acctcfg-SystemMessage.png.
Ok, will commit that one.
In fact, I've been considering a patch to the Trac core for the plugin admin page that is patterned after your inline notices. After enabling or disabling a component, there is a notice at the top of the page, but it is usually out of site since the redirect contains a fragment that lands the user on the plugin for which the action was invoked.
I know what you mean. This has occurred to me many times before as well with similar uneasy feelings.
comment:27 Changed 12 years ago by
(In [12513]) AccountManagerPlugin: Move hash refresh procedure restart message, refs #10741.
I feel that it looks much better now, close to the command button.
comment:28 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
I agree.
In fact I'm already working on related improvements for the account table. But this is WiP and dedicated to adding some more tiny icons like for account locking. Suggestions welcome. A gray-out looks like a good one.