launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/app/javascript/ui/banner.js'
> --- lib/lp/app/javascript/ui/banner.js	2014-05-29 15:02:53 +0000
> +++ lib/lp/app/javascript/ui/banner.js	2016-01-21 19:04:48 +0000
> @@ -37,6 +37,22 @@
>              '</div>'
>          ].join(''),
>  
> +        /**
> +         * Position banners and body appropriately.
> +         *
> +         * @method _set_positions
> +         */
> +        _set_positions: function () {
> +            var banners = Y.all('.yui3-banner-content');
> +            banners.each(function (banner, i) {
> +                banner.setStyle('top', (i * 47) + 'px');
> +            });
> +            var location_bar = Y.one('body #locationbar');
> +            if (Y.Lang.isValue(location_bar)) {
> +                location_bar.setStyle(
> +                    'padding-top', (banners.size() * 47) + 'px');
> +            }
> +        },

This could also just set appropriate classes on the body. If this way is cleaner, there should at least be a comment on both ends that the CSS duplicates the JS rules.

>  
>          /**
>           * Bind events that our widget supports such as closing the banner.


-- 
https://code.launchpad.net/~cjwatson/launchpad/float-banners/+merge/283537
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.