DoubleYou Posted May 5, 2015 Posted May 5, 2015 Discussion topic: Template:Clear Purpose & Usage Performs a clear to force content to appear directly below floating content. Current implementation by S4N is {{#ifeq: {{{1|}}} | left | {{#vardefine:clear|left}} }} {{#ifeq: {{{1|}}} | right | {{#vardefine:clear|right}} }} {{#if: {{{1|}}} | | {{#vardefine:clear|both}} }}<div style="clear: {{#var:clear}}"> </div> Wouldn't it be much simpler to have it be... <div style="clear: {{{1|both}}};"></div> Or is there some reason we only want to allow the left and right values? Modified s4n's original implementation to include a 'both' parameter to reflect the default (... so {{clear}} and {{clear|both}} do the same thing).
z929669 Posted May 5, 2015 Posted May 5, 2015 Behavior is actually different for all three. I used both in the WB and MO guides I think ... maybe all three. It helps to be able to place images on both left and right while allowing text to flow correctly in either situation.
DoubleYou Posted May 5, 2015 Author Posted May 5, 2015 Behavior is actually different for all three. I used both in the WB and MO guides I think ... maybe all three. It helps to be able to place images on both left and right while allowing text to flow correctly in either situation.Ah... yes... you didn't understand my question. I'm suggesting that my code is simpler and does the same job.
z929669 Posted May 5, 2015 Posted May 5, 2015 Ah... yes... you didn't understand my question. I'm suggesting that my code is simpler and does the same job.But I am saying, 'yes' there is good reason why we would only want either left or right values. I also think that we can simplify the template, since we are only talking about changing one attribute value amongst three values. I think named parameters method works well for this case (including default value after the pipe):<div style="clear: {{{method|both}}}"></div>... calling like ...{{clear|method=left}} {{clear|method=right}} {{clear|method=both}} {{clear}}The last call without the parameter defined will substitute the default 'both' due to the template parameter containing this default value.
DoubleYou Posted May 5, 2015 Author Posted May 5, 2015 So basically you agree with me, except we should change 1 to 'method'
z929669 Posted May 5, 2015 Posted May 5, 2015 Yes, I guess that's true ... I get caught up in semantics ;) I am not sure why it uses a complex method now, but I assume it was early training in wiki coding for s4n ... testing usage of parserfuctions. Making these changes will require many template calls (except for "{{clear}}") to be updated, so I will see about it at some point.
DoubleYou Posted May 5, 2015 Author Posted May 5, 2015 You should be able to just use a replace for all{{{clear|to be{{{clear|method=
z929669 Posted May 5, 2015 Posted May 5, 2015 Yep, that's true, but global replace can be tricky and devastating unless you go through it and review potential changes verify carefully. At the moment, the template works as expected, so it is relatively low priority ... now if transclusion limits are a big issue, this change could make a big difference, but I don't think it is an issue outside of a couple User guides, no?
DoubleYou Posted May 5, 2015 Author Posted May 5, 2015 It's not an issue on any page at the moment, and the page that was experiencing it will need to nearly double before it does so again. Besides that, it doesn't even use this template. I just like the templates being as simple as possible.
stoppingby4now Posted May 13, 2015 Posted May 13, 2015 The code you suggest DoubleYou allows any value to be passed in, not just "left" or "right", with a default case to "both". It may be less code, but it ends up allowing for bogus HTML. I don't see forcing a "method=" identifier giving any real benefit. As has been mentioned, globally replacing all occurrences can be tricky, and I don't see any significant gain when compared to folks needing to learn a new way of using it.
z929669 Posted May 13, 2015 Posted May 13, 2015 Yeah, I now have a preference for named arguments, due to the potential URL issues ... not applicable in this case, but I think it is a good "best practice" for new templates in general. The way it is now is probably even better with respect to functional specificity, even though it is more complex and harder to follow. I would prefer to use a consistent method for all templates whichever way you think is good best practice in various situations.
stoppingby4now Posted May 13, 2015 Posted May 13, 2015 Could you expand on what you mean by potential URL issues? Do you mean when passing them as a parameter? In general, I also like named parameters. The caveat to that is for simple templates that only take one or two parameters. Named parameters are pretty much a necessity when dealing with large templates (in terms of number of parameters that can be passed in), but I'm also a minimalist where it makes sense and prefer the road to less typing. This template would fall into my caveat bucket. It only accepts one parameter or none, and in context it still makes sense without a named parameter. Less typing is a bonus. The one thing that could be added is to accept the key word "both" along with nothing to do the full clear to provide consistency of intent. I'll also add that it's also fairly common, even for large templates, to have a mixture of both named and non-named parameters to a template. In these cases, the non-named are used to specify required arguments, and are usually small in number (say 1-3). The named parameters are then used for optional arguments.
TechAngel85 Posted May 13, 2015 Posted May 13, 2015 URLs were not being parsed correctly inside some templates such as the Notice and Warning templates until Z changed them. I think that is what he meant. In fact, most of the wiki code wasn't being parsed correctly.
z929669 Posted May 13, 2015 Posted May 13, 2015 Correct .... @s4nsee note #2 under Anonymous Parameters
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now