launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:sassy-css-no-op-css-compilation into launchpad:master with ~twom/launchpad:sassy-css-install-node-sass as a prerequisite.

Commit message:
Move to node-sass for CSS concatenation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/384706

Use node-sass to combine and minify our CSS.
Also remove the combinecss file and entry point as we're not using it anymore.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:sassy-css-no-op-css-compilation into launchpad:master.

diff --git a/Makefile b/Makefile
index 2283d58..f7ba51a 100644
--- a/Makefile
+++ b/Makefile
@@ -182,7 +182,7 @@ css_combine: jsbuild_widget_css
 	${SHHH} bin/sprite-util create-image
 	${SHHH} bin/sprite-util create-css
 	ln -sfn ../../../../yarn/node_modules/yui $(ICING)/yui
-	${SHHH} bin/combine-css
+	SASS_BINARY_PATH=$(NODE_SASS_BINARY) $(YARN) run node-sass --output-style compressed --include-path $(WD)/$(ICING) --follow --output $(WD)/$(ICING) $(WD)/$(ICING)/combo.scss
 
 jsbuild_widget_css: bin/jsbuild
 	${SHHH} bin/jsbuild \
diff --git a/lib/canonical/launchpad/icing/combo.scss b/lib/canonical/launchpad/icing/combo.scss
new file mode 100644
index 0000000..626d1aa
--- /dev/null
+++ b/lib/canonical/launchpad/icing/combo.scss
@@ -0,0 +1,41 @@
+@import 'css/base',
+'ubuntu-webfonts',
+'style',
+'yui/cssreset/cssreset',
+'yui/autocomplete-list/assets/skins/sam/autocomplete-list',
+'yui/calendar-base/assets/skins/sam/calendar-base',
+'yui/calendar/assets/skins/sam/calendar',
+'yui/calendarnavigator/assets/skins/sam/calendarnavigator',
+// Since the old cssgrids uses yui-, and the new uses yui3-, it is only
+// used for the calendar.
+'yui/cssgrids/cssgrids',
+// Use the old cssgrids instead of the new cssgrids.
+'cssgrids/grids',
+'build/ui/assets/skins/sam/lazr',
+'build/ui/assets/skins/sam/banner',
+'build/inlineedit/assets/skins/sam/editor',
+'build/overlay/assets/skins/sam/pretty-overlay',
+'build/formoverlay/assets/formoverlay-core',
+'build/inlinehelp/assets/inlinehelp-core',
+'build/indicator/assets/indicator-core',
+'build/confirmationoverlay/assets/confirmationoverlay-core',
+'build/picker/assets/skins/sam/picker',
+'build/activator/assets/skins/sam/activator',
+'build/choiceedit/assets/choiceedit-core',
+'build/ordering/assets/ordering-core',
+'build/gallery-accordion/assets/gallery-accordion-core',
+'build/gallery-accordion/assets/skins/sam/gallery-accordion-skin',
+'build/inline-sprites-1',
+'build/inline-sprites-2',
+'build/block-sprites-1',
+// Include our main stylesheets at the end so they
+// take precedence over the others.
+'css/base',
+'css/typography',
+'css/colours',
+'css/forms',
+'css/layout',
+'css/modifiers',
+// This shouldn't require the _index, but doesn't appear to be allowable
+// in this version of node-sass
+'css/components/_index';
diff --git a/lib/canonical/launchpad/icing/css/components/_index.scss b/lib/canonical/launchpad/icing/css/components/_index.scss
new file mode 100644
index 0000000..e1623c8
--- /dev/null
+++ b/lib/canonical/launchpad/icing/css/components/_index.scss
@@ -0,0 +1,8 @@
+@import 'batch_navigation',
+        'bug_picker',
+        'profiling_info',
+        'sidebar_portlets',
+        'bug_listing',
+        'portlets',
+        'sharing',
+        'yui_picker';
diff --git a/lib/lp/app/templates/base-layout-macros.pt b/lib/lp/app/templates/base-layout-macros.pt
index 6d9be5d..938d47d 100644
--- a/lib/lp/app/templates/base-layout-macros.pt
+++ b/lib/lp/app/templates/base-layout-macros.pt
@@ -190,7 +190,7 @@
   <tal:comment replace="nothing">
     This macro loads a single css file containing all our stylesheets.
     If you need to include a new css file here, add it to
-    lib/lp/scripts/utilities/js/combinecss.py instead.
+    lib/canonical/launchpad/icing/combo.scss instead.
 
     We load the CSS from the same host that served the HTML in order to optimize
     IE caching over SSL. This is inefficient when you cross subdomains (from
diff --git a/lib/lp/scripts/utilities/js/combinecss.py b/lib/lp/scripts/utilities/js/combinecss.py
deleted file mode 100755
index 885d11c..0000000
--- a/lib/lp/scripts/utilities/js/combinecss.py
+++ /dev/null
@@ -1,83 +0,0 @@
-# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-from __future__ import absolute_import, print_function, unicode_literals
-
-import os
-
-import scandir
-
-from lp.scripts.utilities.js.combo import combine_files
-from lp.scripts.utilities.js.jsbuild import ComboFile
-from lp.services.config import config
-
-# It'd probably be nice to have this script find all the CSS files we might
-# need and combine them together, but if we do that we'd certainly end up
-# including lots of styles that we don't need/want, so keeping this hard-coded
-# list seems like the best option for now.
-names = [
-    'ubuntu-webfonts.css',
-    'style.css',
-    'yui/cssreset/cssreset.css',
-    'yui/autocomplete-list/assets/skins/sam/autocomplete-list.css',
-    'yui/calendar-base/assets/skins/sam/calendar-base.css',
-    'yui/calendar/assets/skins/sam/calendar.css',
-    'yui/calendarnavigator/assets/skins/sam/calendarnavigator.css',
-    # Since the old cssgrids uses yui-, and the new uses yui3-, it is only
-    # used for the calendar.
-    'yui/cssgrids/cssgrids.css',
-    # Use the old cssgrids instead of the new cssgrids.
-    'cssgrids/grids.css',
-    'build/ui/assets/skins/sam/lazr.css',
-    'build/ui/assets/skins/sam/banner.css',
-    'build/inlineedit/assets/skins/sam/editor.css',
-    'build/overlay/assets/skins/sam/pretty-overlay.css',
-    'build/formoverlay/assets/formoverlay-core.css',
-    'build/inlinehelp/assets/inlinehelp-core.css',
-    'build/indicator/assets/indicator-core.css',
-    'build/confirmationoverlay/assets/confirmationoverlay-core.css',
-    'build/picker/assets/skins/sam/picker.css',
-    'build/activator/assets/skins/sam/activator.css',
-    'build/choiceedit/assets/choiceedit-core.css',
-    'build/ordering/assets/ordering-core.css',
-    'build/gallery-accordion/assets/gallery-accordion-core.css',
-    'build/gallery-accordion/assets/skins/sam/gallery-accordion-skin.css',
-    'build/inline-sprites-1.css',
-    'build/inline-sprites-2.css',
-    'build/block-sprites-1.css',
-    # Include our main stylesheets at the end so they
-    # take precedence over the others.
-    'css/base.css',
-    'css/typography.css',
-    'css/colours.css',
-    'css/forms.css',
-    'css/layout.css',
-    'css/modifiers.css']
-
-
-def main():
-    icing = os.path.join(config.root, 'lib/canonical/launchpad/icing')
-    target = os.path.join(icing, 'combo.css')
-
-    # Get all the component css files so we don't have to edit this file
-    # every time a new component is added.
-    component_dir = 'css/components'
-    component_path = os.path.abspath(os.path.join(icing, component_dir))
-    for root, dirs, files in scandir.walk(component_path):
-        for file in files:
-            if file.endswith('.css'):
-                names.append('%s/%s' % (component_dir, file))
-
-    absolute_names = []
-    for name in names:
-        full_path_name = os.path.abspath(os.path.join(icing, name))
-        absolute_names.append(full_path_name)
-
-    combo = ComboFile(absolute_names, target)
-    if combo.needs_update():
-        result = b''
-        for content in combine_files(names, icing):
-            result += content
-
-        with open(target, 'wb') as f:
-            f.write(result)
diff --git a/setup.py b/setup.py
index acb19b7..9a126c9 100644
--- a/setup.py
+++ b/setup.py
@@ -306,7 +306,6 @@ setup(
                 'lp.services.sitesearch.bingtestservice:main',
             'build-twisted-plugin-cache = '
                 'lp.services.twistedsupport.plugincache:main',
-            'combine-css = lp.scripts.utilities.js.combinecss:main',
             'generate-key-pair = '
                 'lp.services.crypto.scripts.generatekeypair:main',
             'harness = lp.scripts.harness:python',