Maheshjayachandran / closure-library

Automatically exported from code.google.com/p/closure-library
0 stars 0 forks source link

TwoThumbSlider getCssClass should pass orient modifier through getCssName #423

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Steps do reprodice

1. Render a TwoThumbSlider with a CssNameMap set and the orientation fragment 
will not be renamed (minified).

What is the expected output? What do you see instead?

goog.ui.ThoThumbSlider.getCssClass(orient) should return something like 
"a-ab-od" and is instead returning "a-ab-horizontal"

What version of the product are you using? On what operating system?

Closure Library @ Revision 1589

Please provide any additional information below.

Note: If you look in goog.ui.Slider (not TwoThumbSlider), you'll see its 
getCssClass() checks the orient param against 
goog.ui.SliderBase.Orientation.VERTICAL, and calls getCssName with the 
opt_modifier set to 'vertical' or 'horizontal'.
I was confused as to why that is done that way.

Here is the diff to correct the issue on goog.ui.TwoThumbSlider.

Index: twothumbslider.js
===================================================================
--- twothumbslider.js   (revision 1589)
+++ twothumbslider.js   (working copy)
@@ -98,7 +98,7 @@
  * @protected
  */
 goog.ui.TwoThumbSlider.prototype.getCssClass = function(orient) {
-  return goog.ui.TwoThumbSlider.CSS_CLASS_PREFIX + '-' + orient;
+  return goog.getCssName(goog.ui.TwoThumbSlider.CSS_CLASS_PREFIX, orient);
 };

Original issue reported on code.google.com by andre.tannus on 17 Feb 2012 at 1:30

GoogleCodeExporter commented 9 years ago
This won't work. getCssName runs before all inlining and optimizations, and the 
second arg here would need to be a string constant for the compiler to be able 
to do the replace. The getCssClass function appears to currently be as correct 
as it's going to get. Its callers need to work differently -- instead of

blah.getCssClass('horizontal')

they need to be

blah.getCssClass(goog.getCssName('horizontal'))

Or alternatively,

goog.getCssName(goog.ui.TwoThumbSlider.CSS_CLASS_PREFIX, 'horizontal')

Original comment by ibmirkin@gmail.com on 17 Feb 2012 at 4:14

GoogleCodeExporter commented 9 years ago
Will fix shortly. Thanks for the report.

Original comment by chrishe...@google.com on 17 Feb 2012 at 4:46

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1593.

Original comment by chrishe...@google.com on 21 Feb 2012 at 5:57