Modify

Opened 11 years ago

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

Accounts.png (21.4 KB) - added by Ryan J Ollos 11 years ago.
th10741-r12500-1.patch (1.8 KB) - added by Ryan J Ollos 11 years ago.
Patch against r12500 of the trunk.
EntireRowGrey.png (13.7 KB) - added by Ryan J Ollos 11 years ago.
th10741-r12501-1.patch (5.0 KB) - added by Ryan J Ollos 11 years ago.
MoreInfo-Expanded.png (6.3 KB) - added by Ryan J Ollos 11 years ago.
MoreInfo-Unexpanded.png (1.6 KB) - added by Ryan J Ollos 11 years ago.
Chrome23-CSS.png (5.0 KB) - added by anonymous 11 years ago.
SystemMessage.png (34.2 KB) - added by anonymous 11 years ago.
20130104_admin_acctcfg-SystemMessage.png (25.6 KB) - added by Steffen Hoffmann 11 years ago.
suggestion: restart-confirmation replaced right above dedicated button

Download all attachments as: .zip

Change History (37)

comment:1 Changed 11 years ago by Steffen Hoffmann

Keywords: web-UI CSS added
Severity: normalminor

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.

comment:2 Changed 11 years ago by Ryan J Ollos

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 11 years ago by Ryan J Ollos

The icons for locked accounts sound very useful, btw. Looking forward to seeing that.

Changed 11 years ago by Ryan J Ollos

Attachment: Accounts.png added

comment:4 Changed 11 years ago by Ryan J Ollos

comment:5 Changed 11 years ago by Ryan J Ollos

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 11 years ago by Ryan J Ollos

Attachment: th10741-r12500-1.patch added

Patch against r12500 of the trunk.

comment:6 Changed 11 years ago by Ryan J Ollos

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 11 years ago by Steffen Hoffmann

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 Changed 11 years ago by Ryan J Ollos

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 in reply to:  8 Changed 11 years ago by Steffen Hoffmann

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 11 years ago by Ryan J Ollos

Attachment: EntireRowGrey.png added

Changed 11 years ago by Ryan J Ollos

Attachment: th10741-r12501-1.patch added

comment:10 Changed 11 years ago by Ryan J Ollos

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 11 years ago by Ryan J Ollos

Attachment: MoreInfo-Expanded.png added

Changed 11 years ago by Ryan J Ollos

Attachment: MoreInfo-Unexpanded.png added

comment:11 Changed 11 years ago by Ryan J Ollos

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 in reply to:  10 Changed 11 years ago by Steffen Hoffmann

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 Changed 11 years ago by Ryan J Ollos

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 11 years ago by anonymous

Attachment: Chrome23-CSS.png added

comment:14 in reply to:  13 Changed 11 years ago by Steffen Hoffmann

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 Changed 11 years ago by Ryan J Ollos

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 in reply to:  15 Changed 11 years ago by Steffen Hoffmann

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 11 years ago by Steffen Hoffmann

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 11 years ago by Ryan J Ollos

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 11 years ago by Steffen Hoffmann

(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 Changed 11 years ago by Ryan J Ollos

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 11 years ago by Ryan J Ollos

Here is a potential issue with [12506]. The system-message wasn't moved when the Password Reset fieldset was moved:

Changed 11 years ago by anonymous

Attachment: SystemMessage.png added

comment:22 Changed 11 years ago by Ryan J Ollos

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 in reply to:  20 Changed 11 years ago by Steffen Hoffmann

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 in reply to:  22 ; Changed 11 years ago by Steffen Hoffmann

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 since add_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 11 years ago by Steffen Hoffmann

suggestion: restart-confirmation replaced right above dedicated button

comment:25 in reply to:  24 ; Changed 11 years ago by Ryan J Ollos

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 in reply to:  25 Changed 11 years ago by Steffen Hoffmann

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 11 years ago by Steffen Hoffmann

(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 7 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.