Posted by realityloop on July 27, 2010 at 5:34am
2 followers
| Project: | Masquerade |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | realityloop |
| Status: | needs review |
Issue Summary
I found a need to do this and thought it might be handy to add
Comments
#1
Patch attached
#2
First, don't abbreviate variables with this module or any other. I thought at first you were checking to see if the site was defaulting to English before deciding whether or not to display a search box.
Since there are only two options, I'd think you could use 0 or 1. The variable itself isn't re-purposed anywhere else, so make it describe what it does, "masquerade_block_search_visibility".
I personally think radio buttons would be better here. Do you have some plans for other options in this box or a reason to need to select more than one?
You tried to stab a kitten with your patch by changing an unrelated string, which would affect translations which have already been submitted. That's a separate issue so the translation maintainers can be notified.
#3
Thanks for the feedback, revised patch attached.
#4
Last patch didn't include uninstall for added variable, please test this one instead.