Message texts should be more like these in core
Standart - October 24, 2007 - 16:43
| Project: | User status change notifications |
| Version: | HEAD |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
Description
I changed the default message texts to conform with core user.module messages.
I also rearranged the forms a bit. The description of the message body for "Activated" was not correct about the reset password. I added a description to the checkbox instead.
| Attachment | Size |
|---|---|
| user_status-forms.patch | 5.48 KB |
#1
#2
I committed a modified version of this patch to DRUPAL-5 and HEAD (and updated the po/user_status.pot translation template -- yay, we just broke all the translations for this module). ;)
A few things:
A) Thanks for the fixes regarding the password reset thing, I guess I missed that when I added that setting. However, I think it's helpful to describe why you'd want to reset the password when the account is activated right next to that setting, not just buried in the description for the activation message body. So, I put the info in both places.
B) While I know core uses the "-- %site team" thing, I personally hate that, and am not going to put that junk in my module's defaults. ;)
C) I also noticed that core never really uses a dot at the end of the titles of checkboxes, so I removed those for even more UI consistency with core.
D) When duplicating the name of another UI element, it's nice to put that in a t() placeholder, since then translators only have to translate that once. For example:
'#description' => t('In addition to the variables described above, %login_url (a one-time login URL) is available in the notification message. If the "%reset_password" box is checked, you may also use %password.', array('%reset_password' => t('Reset password when accounts are activated'))),Thanks for the patch,
-Derek
p.s. You realize this module moved into core for D6, right? I'm not really planning on supporting/maintaining it anymore. The fact that your patch included a fix to broken description is the only reason I applied it at all. ;)
#3
Thanks for committing and improving.
B) I don't like it either but I thought doing it like in core is more important...
D) Thanks for explaining.
Yes, I do know it's in core in D6. I also read the note that you don't want to work on it anymore. I use this module with my d5 project which might be around for some time and I use the setting of admin approval so my users need the feature of this module. I think it's too early to stop the support and I might post an updated translation and more improvements if I need any to share them with others.
#4
Re: (B) these are just the defaults. Sites can re add the silly "-- %site team" if they want.
Re: Continued support for this module in contrib. It's such a simple module that it basically does exactly what it's supposed to do at this point. I'm fairly confident the current functionality is basically bug free. Therefore, I don't expect to need to do much more with it, since it does what it claims to do. If bugs surface, I'll be happy to commit fixes for them for the foreseeable future (I, too, am building some important sites now using 5.x and will probably stick with that for quite a while). I just don't want to add any new functionality directly in this module. If you want it to do other things, chances are good you could either produce your own module that hooks into core in the same way, or form_alter()'s this one to add whatever you need. But, other than minor textual improvements or actual bugs (if any), I'd prefer not to commit any more changes here. Does that make sense?
Cheers,
-Derek
#5
Totally :)
See new issue for "minor textual improvements"...
#6
Automatically closed -- issue fixed for two weeks with no activity.